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

Restore the ability to use %colors to switch terminal theme. #9655

Merged
merged 5 commits into from
Jun 24, 2016

Conversation

Carreau
Copy link
Member

@Carreau Carreau commented Jun 23, 2016

And restore previous coloring of prompts on classical color themes.
Unlike 4.x this will not change with your terminal background.

If the hightligh_style of TerminalInteractiveSHell is set to legacy
then it uses the value of TermianlInteractiveShell.colors to select the
theme:

monokai for darkbg/linux (by decision of BDFL), and old prompt values.
default for lightbg

Closes #9648

@Carreau Carreau added this to the 5.0 milestone Jun 23, 2016
@Carreau
Copy link
Member Author

Carreau commented Jun 23, 2016

@fperez would you like to try/review this ? If this suits you I'll document it.

@@ -324,6 +324,7 @@ def colors(self, parameter_s=''):
%colors nocolor
"""
def color_switch_err(name):
raise ValueError()
Copy link
Member

Choose a reason for hiding this comment

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

Why the change to an exception?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, debug thing.

@fperez
Copy link
Member

fperez commented Jun 23, 2016

Mmh, there's still a bug, though it looks like one we've had since 4.x (but not in 3.x): if I have colors assigned in my config file, then using --colors doesn't work (the prompt color should change below):

image

In 3.x it worked fine (no need to bisect when the bug appeared, but I see it in 4.x too):

image

@fperez
Copy link
Member

fperez commented Jun 23, 2016

BTW, I even think you it would be cool to leave the new default prompt colors unchanged for the "linux" case: the highlight of the prompt numbers is very nice. So we can modernize them a bit, all I want to get is the ability to control the three 'basic' schemes (nothing, dark, light) from the command line including prompts...

In summary:

  • for 'linux', we can use the current green with brighter number default (green/red)
  • for 'lightbg', I'd do something similar, with a brighter/bolder number (blue/red)

And otherwise apply the fixes for --colors to work.

@fperez
Copy link
Member

fperez commented Jun 23, 2016

Confirming the problem about not honoring the command-line override is a traitlets bug, as suspected by @Carreau... Rolling traitlets back from master to 4.0 did the trick. So let's forget about that part as an ipython issue.

@Carreau
Copy link
Member Author

Carreau commented Jun 23, 2016

Confirming the problem about not honoring the command-line override is a traitlets bug, as suspected by @Carreau... Rolling traitlets back from master to 4.0 did the trick. So let's forget about that part as an ipython issue.

The question is was this bug release in any of the stable versions of traitlets ?

@fperez
Copy link
Member

fperez commented Jun 23, 2016

yes, it appeared between 4.0 and 4.1, it's present in 4.1.0.

@Carreau
Copy link
Member Author

Carreau commented Jun 23, 2016

Opened as ipython/traitlets#248

@fperez
Copy link
Member

fperez commented Jun 23, 2016

Great, so the last question is: do you have the patience to make the last modifications I suggested (using the current default colors for linux and a blue/red version of the same --with brighter prompt numbers-- for lightbg)?

We could merge as-is, but I think that will be both cleaner (as it only introduces one new set of prompt colors, not two) and nicer (as it does modernize things a bit and diminishes the mismatch between using --colors and manually selecting highlight styles).

@Carreau
Copy link
Member Author

Carreau commented Jun 23, 2016

Only problem is "monokai" is unreadable on lighbg:

colors

Pushed your required changes and docs.

@willingc do you want to get a looks at the docs additions I made ?

@fperez
Copy link
Member

fperez commented Jun 23, 2016

I think it's OK: the default one is a good compromise for light/dark, where as the explicit request for light/dark themes will optimize specifically for those.

@jbasalone
Copy link

👍

And restore previous coloring of prompts on classical color themes.
Unlike 4.x this will **not** change with your terminal background.

If the hightligh_style of TerminalInteractiveSHell is set to `legacy`
then it uses the value of TermianlInteractiveShell.colors to select the
theme:

monokai for darkbg/linux (by decision of BDFL), and old prompt values.
default for lightbg

Closes ipython#9648
@Carreau
Copy link
Member Author

Carreau commented Jun 24, 2016

Thanks @jbasalone and @fperez. Rebase for conflict and pushed.
@willingc assigned to have a look at the docs.

@@ -20,7 +20,7 @@ Backwards incompatible changes
------------------------------


The `install_ext magic` function which was deprecated since 4.0 have now been deleted.
The `install_ext magic` function which was deprecated since 4.0 have now been deleted.
Copy link
Member

Choose a reason for hiding this comment

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

have -> has

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

Copy link
Member Author

Choose a reason for hiding this comment

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

I give you access to my fork so if needed you can make changes directly.

@willingc
Copy link
Member

@Carreau I'm making the rest of the edits and will push to your branch. Seems simpler :-)

@Carreau
Copy link
Member Author

Carreau commented Jun 24, 2016

@Carreau I'm making the rest of the edits and will push to your branch. Seems simpler :-)

Thanks ! Seems great ! Appreciated !

@Carreau
Copy link
Member Author

Carreau commented Jun 24, 2016

Thanks @willingc !

@Carreau
Copy link
Member Author

Carreau commented Jun 24, 2016

Ok, self merging. I'll try to do an RC this afternoon.

@Carreau Carreau merged commit e8e38b4 into ipython:master Jun 24, 2016
@Carreau Carreau deleted the legacy-colors branch June 24, 2016 19:05
@Carreau Carreau mentioned this pull request Jun 24, 2016
@fperez
Copy link
Member

fperez commented Jun 25, 2016

Thanks everyone, kudos to @willingc for your prompt (no pun intended :) doc work. Love seeing good code landing with proper docs. Next thing you know, our project-wide practices will actually have improved!! ;)

cmft added a commit to cmft/sardana that referenced this pull request Feb 27, 2018
Using Spock with a bright background terminal themes commands are not visible
It seems related with an IPython V5  bug ipython/ipython#9655

Change spock colors to  'Neutral' for IPython >= V5

Fix sardana-org#706
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants