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

Improve situation for linux in-kernel VTs (TERM = "linux") #2311

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@faho
Member

faho commented Aug 17, 2015

This adds a special colorscheme and prompt function (a clone of the "classic" prompt) guaranteed to work
on a VT and activates them automatically if $TERM = "linux".

set_color is overridden to only allow the 8 colors VTs have (under the
assumption those are always the same) and the color variables are
shadowed with global ones so they don't pollute our nice capable terms.

Fixes #2070.

Everyone okay with the approach or should we bend over backwards even harder?

Improve situation for linux in-kernel VTs (TERM = "linux")
This adds a special colorscheme and prompt function guaranteed to work
on a VT and activates them automatically if $TERM = "linux".

set_color is overridden to only allow the 8 colors VTs have (under the
assumption those are always the same) and the color variables are
shadowed with global ones so they don't pollute our nice capable terms.
@faho

This comment has been minimized.

Show comment
Hide comment
@faho

faho Aug 30, 2015

Member

I've now merged this as f71e877 since nobody objected.

Member

faho commented Aug 30, 2015

I've now merged this as f71e877 since nobody objected.

@faho faho closed this Aug 30, 2015

@faho faho deleted the faho:linux-colors branch Aug 30, 2015

set -g fish_color_operator cyan
set -g fish_color_quote blue
set -g fish_color_autosuggestion yellow
set -g fish_color_valid_path

This comment has been minimized.

@zanchey

zanchey Aug 31, 2015

Member

Is there supposed to be an argument here?

@zanchey

zanchey Aug 31, 2015

Member

Is there supposed to be an argument here?

This comment has been minimized.

@faho

faho Aug 31, 2015

Member

There probably should be. The default happens to not break, though.

@faho

faho Aug 31, 2015

Member

There probably should be. The default happens to not break, though.

# Don't allow setting color other than what linux offers (see #2001)
functions -e set_color
function set_color
set -l term_colors black red green yellow blue magenta cyan white normal

This comment has been minimized.

@zanchey

zanchey Aug 31, 2015

Member

Sorry for the late review, but I'm not sure this is actually required; set_color should detect a non-256-colour terminal and adjust behaviour accordingly - although I haven't tested it so I might well be wrong.

@zanchey

zanchey Aug 31, 2015

Member

Sorry for the late review, but I'm not sure this is actually required; set_color should detect a non-256-colour terminal and adjust behaviour accordingly - although I haven't tested it so I might well be wrong.

This comment has been minimized.

@faho

faho Aug 31, 2015

Member

This is more to inform the user - set_color won't set the color (try it with purple), but it won't exit with an error status or print any error message.

@faho

faho Aug 31, 2015

Member

This is more to inform the user - set_color won't set the color (try it with purple), but it won't exit with an error status or print any error message.

@pickfire

This comment has been minimized.

Show comment
Hide comment
@pickfire

pickfire Sep 10, 2015

Contributor

It seems that this has broken fish_vi_mode in framebuffer because fish_vi_mode uses bold for the modes. Sorry for the late reply.

Contributor

pickfire commented Sep 10, 2015

It seems that this has broken fish_vi_mode in framebuffer because fish_vi_mode uses bold for the modes. Sorry for the late reply.

@faho

This comment has been minimized.

Show comment
Hide comment
@faho

faho Sep 10, 2015

Member

See 40df11b - bold actually works, underlines don't break anything and printing colors is... well, printing not setting.

Member

faho commented Sep 10, 2015

See 40df11b - bold actually works, underlines don't break anything and printing colors is... well, printing not setting.

@pickfire

This comment has been minimized.

Show comment
Hide comment
@pickfire

pickfire Sep 10, 2015

Contributor

That fix the bold, but there is an still error for background.

Contributor

pickfire commented Sep 10, 2015

That fix the bold, but there is an still error for background.

@faho

This comment has been minimized.

Show comment
Hide comment
@faho

faho Sep 10, 2015

Member

I could have sworn that background doesn't work, but apparently it does. See b231ab7.

Member

faho commented Sep 10, 2015

I could have sworn that background doesn't work, but apparently it does. See b231ab7.

@pickfire

This comment has been minimized.

Show comment
Hide comment
@pickfire

pickfire Sep 10, 2015

Contributor

_Bingo!_ It is working now.

Good luck and have a nice day!

Contributor

pickfire commented Sep 10, 2015

_Bingo!_ It is working now.

Good luck and have a nice day!

@faho faho added this to the next-2.x milestone Sep 26, 2015

@faho faho added the enhancement label Sep 26, 2015

@faho faho added the release notes label Oct 26, 2015

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