Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add line-height configuration parameter #75

Closed
rauchg opened this issue Jul 7, 2016 · 67 comments · Fixed by #2858
Closed

Add line-height configuration parameter #75

rauchg opened this issue Jul 7, 2016 · 67 comments · Fixed by #2858
Labels
🎨 Type: Enhancement Issue or PR is an enhancement request/proposal for Hyper

Comments

@rauchg
Copy link
Member

rauchg commented Jul 7, 2016

No description provided.

@jasonLaster
Copy link

So, this is the closest I got to figuring out how hterm manages lineheight

https://chromium.googlesource.com/apps/libapps/+/HEAD/hterm/js/hterm_scrollport.js#676

It does not look like line-height is part of hterm's built in prefs

https://chromium.googlesource.com/apps/libapps/+/HEAD/hterm/js/hterm_preference_manager.js#105

@rpunkfu
Copy link
Contributor

rpunkfu commented Jul 23, 2016

Huuray, managed to do it! 🎉 Quite hacky way, but still... Will PR later today :D

screen

@jasonLaster
Copy link

Love it. Happy to review
On Sat, Jul 23, 2016 at 11:29 AM Oskar Cieslik notifications@github.com
wrote:

Huuray, managed to do it! 🎉 Quite hacky way, but still... Will PR later
today :D

[image: screen]
https://camo.githubusercontent.com/c2f61dc1a4ad85c5ea81414f19dd04f1d26db456/687474703a2f2f692e696d6775722e636f6d2f49326a55586f4f2e6a7067


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#75 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAPiYnuBnS9QX-bXlB8vC5QFQC7P5Nuoks5qYjNIgaJpZM4JHRw7
.

@rpunkfu
Copy link
Contributor

rpunkfu commented Jul 24, 2016

Working on it, still fighting with cursor node haha

@spoike
Copy link

spoike commented Aug 4, 2016

Another reason for having a lineHeight config, fonts with powerline symbols do not get proper height by default. The arrow symbol in the screenshow below is higher than the line-height.

screen shot 2016-08-04 at 13 14 13

Input Mono was used for the screenshot above.

@spoike
Copy link

spoike commented Aug 4, 2016

Current workaround is to add the following to the .hyperterm.js configuration for termCSS:

x-row > span {
  line-height: 1em;
  padding: 0.2em 0 0.02em 0;
}

YMMV

Update

The above CSS setting will only affect the line height for x-rowwhich will correct render line height so they align with the powerline symbols. Unfortunately hyperterm sets the position for each line and the cursor as style attribute so the lines will still overlap so this workaround won't work as all users going into this issue will face.

@AliveDD
Copy link

AliveDD commented Aug 15, 2016

config: {
    fontSize: 11,
    lineHeight: 33,
    fontFamily: 'Hack',
    ...
}

... lineHeight doesnt work :(

@kartsims
Copy link

@spoike your workaround doesn't work for me, did you mean adding this to .hyperterm.js as such :

    // custom css to embed in the main window
    css: `
      x-row > span {
        line-height: 2em;
        padding: 0.2em 0 0.02em 0;
      }
    `,

Thanks for your help, I am getting to know HyperTerm and loving it

@spoike
Copy link

spoike commented Aug 16, 2016

@kartsims you're almost there. it's for termCss instead of css.

@colepeters
Copy link
Contributor

@spoike @kartsims Is this the exact block you’re adding, including the backticks instead of default single quotes?

    termCSS: `
      x-row > span {
        line-height: 2em;
        padding: 0.2em 0 0.02em 0;
      }
    `,

I’ve added this to my .hyperterm.js but it doesn’t appear to have changed anything.

@kartsims
Copy link

kartsims commented Aug 20, 2016

When I do add this, the setting is applied (so I see changes to my terminal window).

capture d ecran 2016-08-20 a 11 51 58

BUT it is not enough to fully change the line height everywhere. Some values are fixed in style attributes of some elements

@dbpolito
Copy link

i would love to see this too, the workarounds didn't work for me either.

@spoike
Copy link

spoike commented Sep 13, 2016

Sorry about the confusion and the late reply. The workaround is for fixing the line height of the line which is only relevant if you are using powerline symbols in your shell prompt and other terminal UI, though it won't fix the positioning of the lines (which is currently set directly on the x-row's style attribute IIRC) and they will overlap each other.

I'll edit my reply above to make this clearer.

@piotr-yuxuan
Copy link
Contributor

Just to add my two cents contribution: I use FiraCode with powerline and lines were wrapped due to bad css.
I feel like this setting give a better appearance:

termCSS: `x-row > span {
        line-height: 1.3em;
      }
      `,

@0x80
Copy link

0x80 commented Oct 21, 2016

Is anyone still working on this?

The x-row > span selector only works on console input rows and not the command output text.

@iamstarkov
Copy link
Contributor

i dont think anybody works on this, though check in the chat https://zeit-community.slack.com/messages/hyper/

I would be glad i smb will fix this

@adityavm
Copy link

adityavm commented Oct 21, 2016

this doesn't look like it can be fixed by hyper. it needs to be fixed by hterm, which currently doesn't support anything like a line-height property.


i tried my hand at a super hacky, sorta buggy plugin that achieves this: https://gitlab.com/adityavm/hyper-lineheight. its problems are listed in the readme.

this is one of two methods i tried, the other one being setting the x-row height after calculating based on a provided line height, then calculating the correct top for the cursor whenever there was a cursor event. that implementation however ran into issues with scrolling as hterm has a different character size set internally, so it wasn't able to accurately judge which rows were visible at what time. the cursor would scroll off the bottom with no way to scroll to get to it.

if someone is willing to continue where i left off, i can put up the other code as well.

@rauchg
Copy link
Member Author

rauchg commented Oct 21, 2016

@adityavm yep, accurate observation. We need to improve the hterm character / line measurement logic

@bendc
Copy link

bendc commented Dec 1, 2016

Really looking forward to this -- basically the only visual annoyance I have that prevents me from switching :p

@PezCoder
Copy link

PezCoder commented Dec 3, 2016

No offense but my vim is not operational + neovim looks like shit because of so less padding between lines.
screenshot 2016-12-03 20 53 14

So please, please do something. I love the UI of this terminal & that is what keeping me alive here.

@rauchg
Copy link
Member Author

rauchg commented Dec 4, 2016

Understood. We'll get to it. Thanks for chiming in

@dvdchr
Copy link

dvdchr commented Dec 15, 2016

For an increased line height, I am currently using a font with increased line height by default; for example: https://github.com/andreberg/Meslo-Font. (I am using Meslo LG L)

I also use this to increase the line height in Xcode, and apparently it works fine in Hyper.

@breachofmind
Copy link

Hey all, it appears that all x-row elements height are scaled based on the fontSize property. So I was able to get some line-height without any weird side-effects:

fontSize: 16
// ...
termCSS: 'x-row {font-size:14px;}'

So basically the x-row height - x-row font-size = padding between rows, AKA line-height. You'll have to let me know if that works with all your handsome prompt styles.

@sidd
Copy link

sidd commented Dec 27, 2016

@breachofmind the cursor node renders in the incorrect spot with this method

@adesnmi
Copy link

adesnmi commented Jan 5, 2017

Does anyone know what the status with this is? Is there an update coming soon, would be happy to fork and push a fix if not already in the works.

@rauchg
Copy link
Member Author

rauchg commented Jan 8, 2017

@muyiwaolu I believe @dotcypress has been looking into this problem

@tonatiuh
Copy link

Any news on this? Thanks.

@pencilcheck
Copy link

I would like to know if there is a good solution for this as well, hyper is awesome btw. :D

@hemangsk
Copy link

+1 😃

@im-n1
Copy link

im-n1 commented Sep 16, 2017

Thank you, that look cool. How can I modify all the glyphs at once? Like add some line height.

@cbioley
Copy link

cbioley commented Sep 16, 2017

@grafa I'm definitely not a font guru but what I did was add/remove the same amount from ascent/descent (hamburger menu -> font settings).

@mandeepsinghgill
Copy link

100% will work.

termCSS: '.xterm-rows div {margin: 15px 0}',

@PerStirpes
Copy link
Contributor

{ line-height: 1.25; } specified as a pure number, which is interpreted as relative to the font size of the element.

Read about it =>: Guide to using special characters in HTML: Line spacing problems

@timvandijck
Copy link

I would love to see a solution for this as none of the things mentioned above really helped. I feel like line height is something a lot of people like to configure in terminals.

@im-n1
Copy link

im-n1 commented Sep 28, 2017

@timneutkens The font trick (look at my prev post) works 100% cause it's font thing not the hyper term hack.

@jabacchetta
Copy link

jabacchetta commented Sep 29, 2017

Used hyper for the first time today and loving it... but definitely noticed the line-height. Was hoping to be able to add it to the config.

@Cygnusfear
Copy link

Cygnusfear commented Oct 3, 2017

Thought I fixed it with the above commit but ended up messing something else up. I think this is doing the trick for me now with no further issues, so maybe some other people are willing to confirm?

Should work in 2.0.4:
termCSS: '.xterm {line-height: 17px} .xterm-rows > div {display: block; line-height: 15px}'

@cbioley
Copy link

cbioley commented Oct 3, 2017

@Cygnusfear I just tested a few seconds ago.

  • v. 1.4.8: it does not work
  • v. 2.0.4: it does work (yay!)

Thank you :)

@Cygnusfear
Copy link

Yes, I forgot to mention! It's only going to work with the new 2.0.4 release!! Glad it works! Enjoy!

@ghost
Copy link

ghost commented Oct 29, 2017

@Cygnusfear : What would it be if I use hyper on windows. Seem that config doesn't work on my box.

@jesseleite
Copy link

jesseleite commented Nov 14, 2017

Surprised this isn't a thing in .hyper.js yet 😢

@jesseleite
Copy link

One problem with some of these CSS line-height/margin/padding examples is:

When a text block with a background colour is rendered, you end up with spaces between the lines, rather than padding around the actual text itself.

@im-n1
Copy link

im-n1 commented Nov 16, 2017

@jesseleite
what "line-height" CSS setup u use?

@jesseleite
Copy link

I tried the examples in this thread above. For now I'm just going to use font-line to add line-height into the font file itself. That said, would still be nice if lineHeight becomes a config option in Hyper 👍

@lehelbabos
Copy link

+1 on this.... Love hyper but it'd be great if I could set the lineheight without having to resort to some sort of hack.

@im-n1
Copy link

im-n1 commented Nov 26, 2017

One year later.
Number of line height config options: 0
Number of cursor config options: 3

@estebanrao
Copy link

Hey everyone!!
For all of you using Meslo fonts you can fix the line-height issue by doing:

termCSS: `
  x-row {
    line-height: inherit;
  }
`

@Spaxe
Copy link

Spaxe commented Feb 8, 2018

I'm tracing this issue, having begun to use Fira Code with ligature support (so long Meslo, Input Mono), and changing fonts is not a valid solution for me.

I suspect it's a limitation with features from xterm not exposed properly, not just a hyper issue. ref something like xtermjs/xterm.js#302

@12luisalmeida
Copy link

Any solution for this?

@chrispuska
Copy link

Sadly, no. Neither of the previous examples work, on latest canary at least. You can still change the font line-height directly to make it work, but I think thats a bit too much of a hack.

@cbioley
Copy link

cbioley commented Mar 16, 2018

@12luisalmeida Try to scroll this issue, just a bit ^^

@chabou
Copy link
Collaborator

chabou commented Apr 16, 2018

This workaround will not work with our v2 anymore.
xterm is using canvas now

@Spaxe
Copy link

Spaxe commented Apr 17, 2018

What's the proper way to set line height? I see xterm 3.0 exposes an option at xtermjs/xterm.js#938

@msikma
Copy link

msikma commented Apr 17, 2018

This should probably be closed in favor of #2850 since it targets the new Canvas renderer.

@chabou
Copy link
Collaborator

chabou commented Apr 19, 2018

Yes! Closed in favor of #2850

@chabou chabou closed this as completed Apr 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎨 Type: Enhancement Issue or PR is an enhancement request/proposal for Hyper
Projects
None yet
Development

Successfully merging a pull request may close this issue.