ulfalizer / Kconfiglib

A flexible Python 2/3 Kconfig implementation and library
ISC License
450 stars 161 forks source link

menuconfig: Add support for custom color schemes (aka styles) #53

Closed pinkfluid closed 6 years ago

pinkfluid commented 6 years ago

I've added experimental basic themeing (styling) support to menuconfig.

The code is probably not the greatest, but might be a source of few ideas. I'm sending a pull request just so its easier to view the diff. Please close it at your leisure.

There are two sources of styles:

By default menuconfig starts off with the default theme. The default theme can be changed by using the following keywords:

    path, separator, list, selection, inv_list, inv_selection, help,
    frame, body, edit, jump_to_edit, text

Colors can be define symbolically, for example: "bold_bright_yellow_on_blue"

or using the 256-color scheme:

"underline_color226_on_color33"

A special keyword "style=" is used to use a builtin theme instead of the default.

Examples:

Use aquatic theme:

MENUCONFIG_STYLE="style=aquatic"

Use a theme with a purple accent instead of yellow:

MENUCONFIG_STYLE="selection=underline_bold_white_on_purple separator=white_on_purple"

Aquatic theme with purple accent:

MENUCONFIG_STYLE="style=aquatic selection=underline_bold_white_on_purple separator=white_on_purple"

Kconfig example of the above:

    # $style: style=aquatic
    # $style: selection=underline_bold_white_on_purple
    # $style: separator=white_on_purple

The style is parsed from all soruce Kconfig files, so the theme can reside in its own file and can be sourced at the beginning.

In the future, support for JSON files are something similar can be added.

ulfalizer commented 6 years ago

Instead of having style=..., maybe anything that doesn't have a = in it could be taken as a predefined style, and work the same as inserting all the settings from the style at that location.

You'd then be able to write stuff like MENUCONFIG_STYLE=aquatic and MENUCONFIG_STYLE='aquatic separator=white_on_purple', where the later one overrides a single setting.

The default style would be implicit, so the later example would work the same as MENUCONFIG_STYLE='default aquatic separator=white_on_purple'.

Maybe settings could be separated by ,, like this:

MENUCONFIG_STYLE='aquatic separator=blue_on_red,bold selection=white_on_yellow,standout'

Maybe there could be separate fg:<color> and bg:<color> attributes too, instead of having specialized parsing for colors:

MENUCONFIG_STYLE='aquatic separator=fg:blue,bg:red,bold selection=...'

Does __ do anything special at the module level by the way? Only seen it within classes.

pinkfluid commented 6 years ago

I think that I've addressed all the issues you mentioned above.

The style syntax is now exactly like the one you specified at the end of your comment, for example, the style definition below works as expected:

MENUCONFIG_STYLE='aquatic separator=fg:blue,bg:red,bold'

Regarding the __ vs _. I'm not really sure what's the difference -- in classes I always viewed _ as "use at your own risk symbols' and __ as "don't use here be dragons" symbols.

In classes _ basically does nothing, while __ mangles the symbol name -- however, I'm not sure if that's the case for modules. Anyway I renamed all the __ symbols _ to be consistent with the rest of the code.

ulfalizer commented 6 years ago

Could you merge all the commits into a single commit for future versions? It makes the history cleaner once stuff is merged, and makes things easier to review.

You can use git push --force to push with a rewritten history.

ulfalizer commented 6 years ago

Thinking about it, your design for attributes (including the -1 defaults for colors) makes more sense.

If a later style does not specify bold for example, then you don't want the bold attribute to be inherited, because then there'd be no way to undo it. And if attributes like that won't be inherited, then colors probably shouldn't be inherited either. So foo=... should probably just replace the entire style for foo, like you did it.

Sorry for the noise!

To clarify the design I had in mind, this is what could happen internally if

MENUCONFIG_STYLE="separator=fg:white selection=fg:white,bg:black,bold"

is passed in:

  1. "default" is added to the beginning of MENUCONFIG_STYLE, giving "default separator=fg_white selection=fg:white,bg:black,bold"

  2. The style parsing code runs on the string and sees that default doesn't have a = in it, looks it up as a style, and parses its style string (which is just a normal MENUCONFIG_STYLE string). This fills in fields in a style dictionary, where the default style is guaranteed to specify values for all keys.

  3. The style parsing code next sees the separator=fg:white, and replaces the style at the separator key in the dictionary.

  4. The style parsing sees the selection=fg:white,bg:black,bold and similarly replaces the style at the selection key.

  5. The style dictionary now holds the final style, which is initialized.

You could also have stuff like

MENUCONFIG_STYLE="separator=fg:white selection=fg:white,bg:black,bold aquatic"

, which would give all the attributes specified in the aquatic theme their values from that style.

You'd also be able to reference styles recursively from within styles.

pinkfluid commented 6 years ago

Hi,

I did rebase-squash to a single commit. FYI, I think that github allows you to merge pull request in several different ways -- squash being one of them.

I did change the quotation style and prefixed '_' the global functions.

Regarding your last comment, I believe the code currently already works the way you described.

A style is basically a dictionary of keywords which override the keywords in the parent style (aka template). If no template is selected, the default style is used. If a keyword doesn't exists in the parent style, it is considered an error. This means that the default style also defines the list of valid keywords.

The only special keyword is "style", which selects the template.

The order of keywords in MENUCONFIG_STYLE doesn't really affect the result. Your example above (MENUCONFIG_STYLE="separator=fg:white selection=fg:white,bg:black,bold aquatic) is literally translated to the dictionary below:

{
    "style": "aquatic",
    "separator": "fg:white"
    "selection": "fg:white,bg:black,bold"
}

What happens internally is this:

MENUCONFIG_STYLE                    _AQUATIC_STYLE = \ _                            _DEFAULT_STYLE = \
{                                   {                                               {
    "style": "aquatic",                 "style": "default",
                                        "path": "fg:cyan,bg:blue,bold",  ============>  "path": "fg:black,bg:white,bold",
    "separator": "fg:white",   ======>  "separator": "fg:white,bg:cyan,bold",  ======>  "separator": "fg:black,bg:yellow,bold",
                                                                                        "list": "fg:black,bg:white",
    "selection": "fg:white,bg:black,bold",  =========================================>  "selection": "fg:white,bg:blue,bold",
                                                                                        "inv_list": "fg:red,bg:white",
                                                                                        "inv_selection": "fg:red,bg:white",
                                        "help":  "fg:cyan,bg:blue,bold", ============>  "help": "fg:black,bg:white,bold",
                                        "frame": "fg:white,bg:cyan,bold",============>  "frame": "fg:black,bg:yellow,bold",
                                        "body": "fg:white,bg:blue", =================>  "body": "fg:white,bg:black",
                                        "edit": "fg:black,bg:white", ================>  "edit": "fg:white,bg:blue",
                                                                                        "jump_to_edit": "fg:white,bg:blue",
                                                                                        "text":  "fg:black,bg:white"
}                                   }                                               }

So basically that means that the default style is used as the template for aquatic, which is used as the template for the MENUCONFIG_STYLE.

This is the end result:

2018-09-01-214940_954x430_scrot

pinkfluid commented 6 years ago

FYI I've been also investigating RGB color mode (for example, you could specify fg:#FFFFFF) and it is a bit trickier than initially thought. But I have experimental code that is able to use RGB mode if available, otherwise it can map a RGB value to the standard 256 colors (minus the first 16 colors as they are user defined and there's no way to query their RGB values).

There's also another color mode referred to as "true color" where you're able to use RGB colors without overwriting the current color scheme, but unfortunately ncurses doesn't support it.

ulfalizer commented 6 years ago

Hello,

Currently, there are two different ways to represent a style: Either as a dictionary (in e.g. _DEFAULT_STYLE), or as a string (in MENUCONFIG_STYLE).

I think just the string representation would be enough. That way, the dictionary merging code would disappear, and there'd just be a single style parsing function that handles everything.

What do you think about something like this?

# Predefined styles
# [Maybe the documentation for the styles could be moved to here]
_STYLES = {
    "default": """
    path=fg:black,bg:white,bold
    separator=fg:black,bg:yellow,bold
    list=fg:black,bg:white
    selection=fg:white,bg:blue,bold
    ...
    """,

    "monochrome": """
    path=bold
    separator=bold,standout
    ...
    """,

    "aquatic": """
    path=fg:cyan,bg:blue,bold
    ...
    """,
}

def _parse_style(s):
    for setting in s.split():
        if "=" not in setting:
            # Recursive reference to other style
            _parse_style(_STYLES[setting])
        else:
            key, value = setting.split("=")
            ...
            # Update the style for 'key' (not a constant, so lowercase)
            _style[key] = *new setting*

def _parse_styles():
    # Always parse the default style first. It's guaranteed to fill in all
    # keys.
    _parse_style("default" if curses.has_colors() else "monochrome")

    # Parse any other style settings
    if "MENUCONFIG_STYLE" in os.environ:
        _parse_style(os.environ["MENUCONFIG_STYLE"])

def _init_styles():
    ...
    _parse_styles()
    # Initialize things from the global _style dictionary...

Maybe _style could be local instead of global too, and be returned by _parse_styles(). Stuff like that could always be tweaked later though.

pinkfluid commented 6 years ago

The string representation could work, the only issue that I foresee is that the _parse_styles() function should somehow avoid infinite recursions (style A references style B which references styles C, which references style A).

I'll give it some thought, meanwhile I pushed a new commit that fixes the issues mentioned above.

ulfalizer commented 6 years ago

The string representation could work, the only issue that I foresee is that the _parse_styles() function should somehow avoid infinite recursions (style A references style B which references styles C, which references style A).

I'll give it some thought, meanwhile I pushed a new commit that fixes the issues mentioned above.

Thanks for the fixes!

Could pass a list of all the parent styles to _parse_style() to detect infinite recursion. Probably not a huge deal to leave it out for an initial version though, since it's so obscure.

ulfalizer commented 6 years ago

Thought of another thing that might be nice:

When the _style dictionary or the like is filled in, any kind of key could be accepted, with unknown keys just never getting used later. That way, you'd get forwards and backwards compatibility, which is nice.

pinkfluid commented 6 years ago

Ok, I've got rid of dictionaries. Styles are now represented as strings. A dictionary form is used only when merging the style template with the current style. The recursion bug is there, but python gracefully crashes if that happens.

I've also implemented a _parse_styles() function that parses the string style representation.

The style options were better documented.

pinkfluid commented 6 years ago

Ok, I believe I have addressed the latest batch of fixes. Thank you for reviewing this.

I've updated the docstring to describe the styling feature a bit better -- along these lines I did update the commit message to reflect the global status of the change.

There are two changes that you did not request:

  1. I renamed the _style() function to _style_attr() as it was conflicting with the global _style dictionary
  2. I've changed the "body" foreground color of the aquatic style from "white" to "brightwhite" as (imho) looks a bit better
pinkfluid commented 6 years ago

Fixed the typos and monochrome terminals. Pushed new version.

ulfalizer commented 6 years ago

Finally merged. Thanks a lot!

Should be pushing out a new version with it soon.

ulfalizer commented 6 years ago

I made a small tweak to it to make non-predefined colors a bit easier to use: https://github.com/ulfalizer/Kconfiglib/commit/0bb65bc5628c7493897bf2f8cc786bcbbbcdefe5

Any objections?

pinkfluid commented 6 years ago

Finally, thank you very much :)

One more thing... I've been investigating the possibility of adding RGB color support (for example, by using HTML notation -- fg:#ffffff,bg:#000000) and it seems to be more complicated than I initially thought.

There are basically 4 color modes on modern terminals:

16 color mode is self explanatory, while TrueColor mode is not support by ncurses, so lets ignore it for now.

256 color mode is basically an extension to the 16 color mode where the first 16 colors can be customized, while the colors above 15 are pre-defined.

Colors from 16-231 are a 6x6x6 RGB cube (values from 0-5 don't exactly translate to a RGB value from 0-255, but lets leave this for now). The colors from 232-255 is grey scale palette.

RGB mode is an extension to the 256 color mode. You can basically take any color from 0-255 and define its RGB value. So you can use 16 million colors, but only 256 at a time. This is pretty much how old VGA graphics card used to work.

So there are two options how to do this:

  1. Support only 256 color mode - take a RGB value and map it to the closest color in the 256 color space. This is probably the simplest mode. For example, tmux uses this option.
  2. Support RGB mode - this option needs option 1) as fallback for terminals that do not support RGB mode. What makes it even more complicated is that you cannot mix the 256 color notation (colorXXX) with the RGB color notation (#RRGGBB) as the RGB notation will be overwriting colors in the 256 color space. To have both, you basically need to emulate 256-color mode with RGB mode (colorXXX -> convert to the RGB value -> allocate RGB color).

Now, option 2. sounds complicated but I already have python code that is able to do that (256 emulation + RGB allocation). I compared it to tmux and the results are comparable if not slightly better (mainly because tmux converts a color to grey scale by averaging the R,G,B values, which is not exactly correct).

The real question is -- is this something you might want in kconfiglib? My rough estimate is that option 1) is about 50-80 LOC, while option 2 is 150-200.

Frankly, this feature may not be worth the extra LOCs needed to implemented it, but if you feel it is something that might be useful, I can share the code or maybe come up with another pull request. Option 1 should be simple enough though.

Please share your thoughts.

pinkfluid commented 6 years ago

Regarding the colorNNN -> N change, no objections, looks good.

ulfalizer commented 6 years ago

Added an error-related change too: https://github.com/ulfalizer/Kconfiglib/commit/4e25d1c713b0c37feb6a8781a92b62a52ee5eb04

Thought it might be mean to not point out missing styles/style templates at least, even if it's not an error.

ulfalizer commented 6 years ago

Ahh, sorry, didn't see the longer message above until now. Will look soon. :)

ulfalizer commented 6 years ago

Yeah, terminals are surprisingly messy for a grid of text...

I dug around a bit. Are you sure there are terminals that support 256 colors but only allow you to change the first 16? The 256-color stuff is an xterm extension originally and not implemented on any real hardware terminals, so it seems a bit arbitrary to limit changing colors to the first 16 colors if you allow changing colors at all.

On the curses side, there's just has_colors() for checking if you can use colors, and can_change_color() for checking if you can modify the palette entries (and COLORS for checking how many colors are in the palette).

Could take a look at whatever approach you prefer if you put together a proof-of-concept. Weekend mode initiated now though, so might be off for a while. :)

pinkfluid commented 6 years ago

No, I didn't meant you can change only the first 16 colors, but the other way around :)

In RGB mode you can change all colors from 0..curses.COLORS. The problem with the first 16 colors is that typically users customize those. And there's no way to query the current RGB value of a colors.

The colors from 16-255 can be calculated using a formula (or it might be that curses.color_content() can be used for that, so we can just build a table at startup), so even if we overwrite those RGB values we can always recalculate them.

The algorithm would roughly look like this:

# Skip first 16 colors
num_rgb_colors = 15

def get_colorrgb(r, g, b):
        global num_rgb_colors
        num_rgb_colors += 1
        curses.init_color(num_rgb_colors, r, g, b)
        return num_rgb_colors

# Get a color from the standard 256 color palette
def get_color256(int num):
    if num < 16: return num

   (r, g, b) = caluclate_rgb_from_standard_color_using_formula(num)
   return get_colorrgb(r, g, b)

I'll send a pull request in the following days; but as I said, it could be too much code for just a little feature :)