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

Fish should hard-code italics and dim capabilities on MacOS/xterm-256color #4436

Closed
floam opened this Issue Sep 29, 2017 · 4 comments

Comments

Projects
None yet
3 participants
@floam
Copy link
Member

floam commented Sep 29, 2017

set_color -i; echo -n foo and set_color -d; echo -n foo both do not work in Terminal.app or iTerm due to MacOS shipping a really old terminfo, despite Terminal.app and iTerm supporting the features.

There is an open radar report I filed with Apple over a year ago. It seems unlikely that Apple will update ncurses - it has shipped with a version from 2013 for years. High Sierra did not touch it.

On this platform, fish should override enter_italics_mode and enter_dim_mode with the correct escape to make the features usable.

@floam floam added the enhancement label Sep 29, 2017

@floam floam added this to the fish 2.7.0 milestone Sep 29, 2017

@floam floam self-assigned this Sep 29, 2017

@floam floam closed this in 635e714 Sep 29, 2017

floam added a commit that referenced this issue Sep 29, 2017

Make italics/dim work on MacOS
Work around ancient terminfo on MacOS by hard-coding the correct escapes.
Fixes #4436.

@zanchey zanchey modified the milestones: fish 2.7.0, fish-3.0 Oct 31, 2017

@zanchey

This comment has been minimized.

Copy link
Member

zanchey commented Oct 31, 2017

This got reverted in Integration_2.7.0, and I don't want to take it for now.

@mvolkmann

This comment has been minimized.

Copy link

mvolkmann commented Oct 31, 2017

Bummer. This seems like a very useful fix. In my new fish article at https://mvolkmann.github.io/fish-article/#Colors (see "To enable the use of") I had to go into quite a bit of detail and include several links to explain how to fix this on a Mac. It would be great if none of that was necessary.

@floam

This comment has been minimized.

Copy link
Member

floam commented Oct 31, 2017

It would be great if none of that was necessary.

I agree, and eventually it will not be. @zanchey isn't the bad guy here!

I don't want to speak for him, but I think what he was referring to with the "not take this for now" thing is us not taking this for 2.7.0 for now - as it was milestoned - 2.7b1 is being rolled as we speak as a first beta. I'd have liked to see this fixed for 2.7 - I'd like to see it in a 2.7 beta - that's not in the cards - but regardless it remains fixed in our 3.0/master branch and I don't predict anyone reverting that so this is on a slow boat towards making it to everyone. (I'd perhaps like to see this make it into a 2.7.1 if one ever exists just because it's a TERRIBLY annoying broken feature on macOS, but no promises as that'd really need buy-in from other developers.)

And the one who reverted this patch from the 2.7 integration branch was me - I had cherry-picked it from 3.0 but some changes made since the source diverged caused it not to work so I immediately reverted it. One would need to re-do the patch for 2.7 to get the same behavior and functional italics/dim. Had I been quicker about coming up with a 2.7-specific patch it's possible I might have been able to sneak this fix in for 2.7.

@zanchey

This comment has been minimized.

Copy link
Member

zanchey commented Oct 31, 2017

Indeed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment