Gruvbox theme: Add true color version.

[rht]

This fixes #282.

gruvbox_dark:

gruvbox_dark24 when color depth is 2**24:

gruvbox_dark24 when color depth is 256 (i.e. wrong setting):

[zulipbot]

Hello @rht, it seems like you have referenced #282 in your pull request description, but you have not referenced them in your commit message description(s). When you reference an issue in a commit message, it automatically closes the corresponding issue when the commit is merged.

Please run git commit --amend in your command line client to amend your commit message description with Fixes #282..

An example of a correctly-formatted commit:

commit fabd5e450374c8dde65ec35f02140383940fe146
Author: zulipbot
Date:   Sat Mar 18 13:42:40 2017 -0700

    pull requests: Check PR commits reference when issue is referenced.

    Fixes #51.

Thank you for your contributions to Zulip!

[rht]

I'd say, the true color version with the right setting has a warmer color, and is closer to the color of the screenshot in https://github.com/morhetz/gruvbox (dark mode).

We probably need to add an assertion that if a truecolor theme is used, you must also use the right color depth.

[rht]

How do I fix https://github.com/zulip/zulip-terminal/pull/998/checks?check_run_id=2373629708#step:9:65 ?

[zee-bit]

How do I fix https://github.com/zulip/zulip-terminal/pull/998/checks?check_run_id=2373629708#step:9:65 ?

You need to add the new theme(i.e. gruvbox_dark24) to the set of expected_complete_themes in test_themes.py. :)

[prah23]

While you're at it, would you please also add a theme alias under THEME_ALIASES in zulipterminal/config/themes.py?

[zee-bit]

@rht Thanks for bringing this up. This looks very nice. 👍
I have tested it across all color-depths and it works just as expected.

One thing we may need to consider is, this theme works best with a color-depth of 2**24, so we may need to specify this somewhere(in the docs, maybe). A better approach might be to implicitly use 2**24 for this theme, unless the user explicitly supplies a different color-depth option?

[neiljp]

@rht Thanks for keeping your eye on this :)

It's been a while since I've looked at these palette rules, so I was expecting we might simply have an extra entry in each style/palette for the 24-bit values. Unfortunately that seems not the case, and I do recall challenges with this in the past.

While gruvbox is the only theme that has a 24-bit version at the moment via this PR, I'd really prefer that we don't require the user to specify a compatible bit-depth to go with the style or just error-out, but instead just make it work transparently.

The two approaches I'd look at would be:

• simplest, if it works: If you run your gruvbox24 theme at 256, does it look close enough - or better - than the existing gruvbox at 256? If so, can we replace the old theme with the values in the new one?
• OK, if urwid doesn't work well enough with the first approach: extend our palette entries to include a non-urwid 24-bit value, which we reduce at runtime to an urwid-compatible one depending on the bit-depth we're running at
  (we could extend the latter to simplify our theme datastructure in any case, given how we handle 1-bit/mono)

There may be a different urwid approach to this, but I've not dug into the code, and this is new and not best documented?

Once this is settled we could add an issue for tracking other potential themes to offer which may render better with this mode - light gruvbox? solarized dark/light?

[rht]

While you're at it, would you please also add a theme alias under THEME_ALIASES in zulipterminal/config/themes.py?

I think gruvbox_dark24 is pretty explicit about what it is. Did you mean to alias it to gruvbox24?

[rht]

simplest, if it works: If you run your gruvbox24 theme at 256, does it look close enough - or better - than the existing gruvbox at 256? If so, can we replace the old theme with the values in the new one?

gruvbox24 with 256 color depth looks worse. The white color is off, if you see my first message of this PR.

[rht]

If you want to reduce boilerplate code, I do think it is possible to write a generic abstract theme function, which can generate a specific theme that translates e.g. white into GRUVBOXWHITE24.

[neiljp]

@rht I saw the screenshots, but wasn't sure if they had a particular meaning, which was why I asked, so thanks for clarifying. Do you think we should take something like the second approach I suggested, then?

If we work with a 24-bit version of gruvbox, then I don't think we need an alias. In any case, the aliases were added for backwards compatibility, so we would want to discuss whether we want to use them as shortcuts rather than for historical benefit only.

I'm not sure how much you want to commit to theme refactoring - we could certainly simply work with a 256/24bit reduction only. That would provide a nice first step from palette is style to a theme spec in any case.

[neiljp]

For the boilerplate, there is various scope for reduction, but I think you'd still need eg "selected": [("white", "black"), (WHITE, BLACK), (WHITE24, BLACK24)] for a given style in a theme. Pairing foreground/background would be optional but potentially cleaner.

[rht]

I will do the refactor in a separate PR. The new theme in this PR works and can be used as data/test for the refactor.

[neiljp]

This looks almost good to go 👍 We could do with a quick update of this in the README, from a quick git-grep of places we mention color depth - and the commit messages don't match the repo style.

[rht]

Updated the commit messages.

[neiljp]

@rht Thanks for getting the ball rolling on this once more 👍 Merged with minor text tweaks ending at dbc7353 🎉