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

Fix the rest of the colors used on Windows so they are more visible #10260

Merged
merged 1 commit into from
Feb 9, 2017

Conversation

segevfiner
Copy link
Contributor

This applies a hack similar to the one in
7b7d560 to change the prompt colors
and exception colors so they are more visible as shown in issue #10238

If this fix is acceptable it would be nice if this is backported to IPython 5 as this is very visible. Every user I see using IPython on Windows sees this and most don't seem to understand that they can change it. And on top of that this is especially annoying when using IPython on a computer you don't own or a VM. Having to reconfigure on each one...

Fixes: #10238

This applies a hack similar to the one in
7b7d560 to change the prompt colors
and exception colors so they are more visible as shown in issue ipython#10238

Fixes: ipython#10238
# relatively few users are likely to alter that, we will use the 'Linux' colours,
# designed for a dark background, as the default on Windows.
if os.name == "nt":
ex_colors.add_scheme(ex_colors['Linux'].copy('Neutral'))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be easier (and more amusing) to make Windows use 'Linux' as the default scheme, even though Linux no longer does?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that the Linux color scheme uses monokai syntax highlighting which is weird.
(IPython/terminal/interactiveshell.py:266-268).

I think what we really want is the Linux color scheme with the Pygments default syntax highlighting with the style overrides used in Neutral. (With fixes for Windows when running on Windows)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should be creating a new name for the defaults we use on Windows. I'd like to hear what @Carreau thinks when he wakes up, though, because he's spent more time than me working on colour stuff.

Copy link
Contributor Author

@segevfiner segevfiner Feb 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like this segevfiner@33a804b?

I guess @Carreau will decide?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't use Windows in a while. I know there've been changes in the console, which is now white background (right) ?. This is IIRC one of the reason of "Neutral" to work both on Light and Dark Background.

But I'll trust a fellow windows user in the choice of colors. I've also tested locally and while it's not the coloring scheme I'm used too (put I'm picky and can have my own). I'm not opposed to making it the default even on non-'nt'

# the prompt colors will be wrong without this
if os.name == 'nt':
style_overrides.update({
Token.Prompt: '#ansidarkgreen',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Carreau what version of pygments is needed for these?

Copy link
Contributor Author

@segevfiner segevfiner Feb 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's actually prompt-toolkit that handles this. They are declared in prompt_toolkit/styles/base.py:39-48 and referenced throughout prompt-toolkit.

I tried using hex colors but prompt-toolkit seems quite stubborn to use the dark colors instead of the light colors...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's actually prompt-toolkit that handles this. They are declared in prompt_toolkit/styles/base.py:39-48 and referenced throughout prompt-toolkit.

Yes, though be careful, we may have custom code that special case OutPromptNum . Though I believe it will not be affected as it it generates OutPrompt Tokens and not consume the. I would have to check.

# relatively few users are likely to alter that, we will use the 'Linux' colours,
# designed for a dark background, as the default on Windows.
if os.name == "nt":
ex_colors.add_scheme(ex_colors['Linux'].copy('Neutral'))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't use Windows in a while. I know there've been changes in the console, which is now white background (right) ?. This is IIRC one of the reason of "Neutral" to work both on Light and Dark Background.

But I'll trust a fellow windows user in the choice of colors. I've also tested locally and while it's not the coloring scheme I'm used too (put I'm picky and can have my own). I'm not opposed to making it the default even on non-'nt'

@takluyver
Copy link
Member

Let's not go changing the default colours on non-Windows systems. The current ones are a hard-fought compromise, and I suspect that any change will quickly lead to more arguments.

@Carreau
Copy link
Member

Carreau commented Feb 8, 2017

Let's not go changing the default colours on non-Windows systems. The current ones are a hard-fought compromise, and I suspect that any change will quickly lead to more arguments.

fair enough.

@Carreau Carreau modified the milestone: 6.0 Feb 9, 2017
@Carreau
Copy link
Member

Carreau commented Feb 9, 2017

Ok let's move forward on this one.

Thanks @segevfiner ! Merging.

@Carreau Carreau merged commit 8e75b59 into ipython:master Feb 9, 2017
@segevfiner
Copy link
Contributor Author

Thanks for merging! 😃

You milestoned this as 6.0. Are you sure you don't want this in 5.x? Considering that 7b7d560 is already in 5.x and this pull request essentially completes it, and also fixes the prompt numbers so they look like they used to in IPython 4 and below.

Since 5 (LTS) is the last version to support Python 2.7, many will be using it for quite a while even after 6.0 is out, maybe even using both versions at the same time since they work with both Python 2 and Python 3. Considering this, it will be nice if IPython 5 on Windows uses the same colors as IPython 6+ on Windows, otherwise it would look quite weird for a user that uses both versions that the colors are different and it's nicer if the user doesn't have to use custom configuration to make them the same (At least as long IPython 5/6 and later use the same highlighting system).

@Carreau Carreau modified the milestones: 5.3, 6.0 Feb 9, 2017
@Carreau
Copy link
Member

Carreau commented Feb 9, 2017

Well, that might push them to upgrade to Python 3 and upgrade to IPython 6 right ?
Let's see if it backports cleanly.

@meeseeksdev backport to 5.x

lumberbot-app bot pushed a commit that referenced this pull request Feb 9, 2017
…y are more visible

This applies a hack similar to the one in
7b7d560 to change the prompt colors
and exception colors so they are more visible as shown in issue  10238

If this fix is acceptable it would be nice if this is backported to IPython 5 as this is very visible. Every user I see using IPython on Windows sees this and most don't seem to understand that they can change it. And on top of that this is especially annoying when using IPython on a computer you don't own or a VM. Having to reconfigure on each one...

Fixes:  10238
@Carreau
Copy link
Member

Carreau commented Feb 9, 2017

Well it seems like MrMeeseeks likes you. See #10281

minrk added a commit that referenced this pull request Feb 10, 2017
@Carreau Carreau added the backported PR that have been backported by MrMeeseeks label Feb 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported PR that have been backported by MrMeeseeks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issue with colors on Windows since IPython 5
3 participants