RichHelpConfiguration changes for rich-click 1.8

Question

RichHelpConfiguration changes for rich-click 1.8

dwreeves opened this issue a year ago · 3 comments

Brain dump on config stuff:

Decide on order of operations for resolving configs (e.g. env_var -> rich_config -> user defined default -> global default), and then document this order of operations.
- It's entirely unclear to me whether env vars take precedence over rich_config, or vice versa. This could also itself be an option that the designers of CLI have control over. The reason it is unclear is because users may want to override config for compatibility and accessibility reasons, e.g. color blindness. See items 5-6 for more info.
Make it so environment variables do not resolve until the RichHelpConfiguration object is created, i.e. lazy load the environment.
Is it possible to create SSoT for defaults and type annotations when relying on both a global config and a config class? I think the answer is possibly no-- or alternatively that such a solution would be too metaprogrammatic to be developer friendly, so "no" with extra steps. But, we should at least have some sort of unit-test in place that enforces these things never fall out of sync. Something like:
- assert get_annotation(RichHelpConfiguration, config_option.lower()) is get_annotation(rich_click, config_option.upper()).
- assert getattr(RichHelpConfiguration(), config_option.lower()) is getattr(rich_click, config_option.upper()).
I think we should be making it so rich_click.rich_click is only for global config stuff, and move out all the parsing stuff. There are a few benefits to having a file devoted to global configuration:
- It avoids conceptual errors in the code: none of the help formatting stuff should ever rely on global config options. By isolating the global config in a separate file and then never importing the global config into the file that parses the help stuff, we can easily avoid conceptual errors.
- One file should focus on one thing; right now that file does two things.
- This makes it so the entirety of rich_click.rich_click is itself a public interface, rather than just some of it.
- rich_click.rich_cick module is far too entrenched and has too much a public-facing API to make changing the namespace on users worthwhile. Also, import rich_click as click; click.rich_click.SOME_OPTION = ...; is a perfectly reasonable API. What I am saying is, if you want to separate out the file into two different files, the only choice is to move the formatting stuff rather than moving the global config stuff.
Make it so each (or some subset?) config option can be set with RICH_CLICK_*
- I am also thinking you'd want to deprecate all the other env vars as well. So only RICH_CLICK_* is a valid choice. The obvious exception being the "force terminal" stuff that hooks into systems like Github Actions.
- I am also also thinking you can make RICH_CLICK_* chosen programmatically in case e.g. someone ever wants to do TYPER_* or whatever.
- I think "some subset" is probably more sensible than "all" config options. For example:
  - I imagine setting a color via the env can help in situations where a particular terminal environment makes reading a certain color difficult. So export RICH_CLICK_STYLE_OPTION = "red" could be OK.
  - It's unclear why the config option DEPRECATED_STRING should be settable via the environment, though. End users do not need to be modifying things such as this.
  - If we do this-- Add parsing logic for environment variables: ints, floats, etc. should probably resolve "safely." json.dumps() maybe should also be supported for dicts/lists. We'll see.
Possibly add ignore_environment to rich_help_config? (Default false, probably?). This is a special option, the only one not set via the environment, and needs to be parsed first before everything else.
Allow dicts to be passed into rich_config(help_config=...).
Allow somehow for configuring rich_config(help_config=...) to be either "merge" or "overwrite." Right now, it only ever overwrites, but there should be a way to merge, too. This can be useful in contexts where a RichCommand is inheriting from a RichGroup but there is one special option for the command. Right now you'd need to pass in the entire config again with a single change. (or perhaps it's worse? I think you'd actually need to create a copy() of the object first?).

None of this is really road-mapped just yet for version 1.8. I am just thinking out loud.

I think some of these ideas are no-brainers (1-3, 7), some of these could be controversial (5-6), and then 4 + 8 are somewhere in between no-brainer and controversial.

Basically, I think there is a lot more room for rich_click to start considering how the configuration stuff is managed (from how it works to how it is documented).

Answer 1 · 2023-10-15T19:31:02.000Z

I'm working on some config changes right now.

I was able to remove the reliance on a global formatter, which I'm really happy about. Everything is imperative.

Building the config from rich_click.rich_click is done via a classmethod now, and only needs to be called inside RichHelpFormatter.__init__.

base_config = RichHelpConfiguration.build_from_globals()

That said, it is configured in a way where users may use it:

    @classmethod
    def build_from_globals(cls, module: Optional[ModuleType] = None, **extra: Any) -> "RichHelpConfiguration":

Another thing I have: rich_help_config can now take in a dict:

import rich_click as click

@click.group():
@click.rich_config(help_config={"style_option": "red"})
def cli():
    ...

@cli.command()
@click.rich_config(help_config={"style_argument": "red"})
def subcommand():
    ...

I also intend on having this be merge-able. So in the above case, subcommand by default will have both style_option and style_argument be red, but cli has style_option as red, whereas the default is applied for style_argument. Making it merge-able is not hard; I am however struggling with something more conceptual and weird (see next section below).

Here is what I am struggling with.

Basically, I don't know how multiple config settings from different sources should resolve. Here is what I have so far in the docs which are also work in progress:

# Configuration

`rich-click` adheres to the following lookup order when searching through sources for configuration values (the first source is always applied first, the second source is applied second, etc.):

1. The help config directly passed to the `RichCommand`.
2. The help config of a parent command.
3. Globals in `rich_click.rich_click`.

(Note, even this is tentative-- just the first thing I ended up writing down...)

But now take the above example, except replace the dicts with RichHelpConfiguration instances:

import rich_click as click

# Default is `bold cyan` for both style argument and style option
click.rich_click.STYLE_ARGUMENT = "yellow"
click.rich_click.STYLE_OPTION = "green"

@click.group():
@click.rich_config(help_config=click.RichHelpConfiguration(**{"style_option": "red"}))
def cli():
    ...

@cli.command()
@click.rich_config(help_config=click.RichHelpConfiguration(**{"style_argument": "red"}))
def subcommand1():
    ...

@cli.command()
@click.rich_config(help_config={"style_argument": "blue"})
def subcommand2():
    ...

What should happen here? It's not clear at all. Here are some choices:

cli has style_argument as bold cyan or yellow (which?)
subcommand1 has style_option as any of green, red, or bold cyan (which?)
subcommand2 has style_option as any of green, red, or bold cyan (which?)

And I don't see the majority of these configurations as mutually exclusive. For example, dicts can merge but perhaps RichHelpConfiguration overwrites.

Perhaps users are supposed to use RichHelpConfiguration.build_from_globals() if they want style_argument="yellow" and style_option="green", rather than the kwarg defaults. I think that's reasonable.

But then what to make of merging?

It is also really really hard to satisfying all of the following:

Not too many mypy hacks / ignores.
Autocompletion in an IDE (that also doesn't yell at you at any point) for all interfaces.
Some capability, somewhere, to merge configs of subcommands into either the parent or global config. Even if it is not default.

For example, I have tried using __new__(cls, **kwargs) inside RichHelpConfiguration to be able to tell explicitly which config options the user is using, and which are not being set. E.g. setting style_option="bold cyan" is different than using the default of "bold cyan" for purposes of merging. Something like this:

def __new__(cls_: type, **kwargs: Any):

    fields = set()
    for k in list(kwargs):
        if k in cls_.__dataclass_fields__:
            fields.add(k)
            del kwargs[k]
    new_cls = super().__new__(cls, **kwargs)
    new_cls.__fields_set_by_user__: FrozenSet[str] = frozenset(fields)  # type: ignore[misc, attr-defined]
    return new_cls

But my IDE (PyCharm) ends up using the signature of __new__ rather than the dataclass's __init__. Very annoying! There are too many config options to not have autocomplete. Perhaps this is not the solution. But still! I really want a way to merge configs. Even if that is not the default behavior of RichHelpConfiguration, that's fine. I am also struggling to implement a metaclass version of this that meets my 3 criteria; and a lot of the dataclass augmentation stuff is not available in versions of Python prior to 3.10. I feel though as I delve into the possibility of using a metaclass that I may be "losing the plot" so to speak.

Another example I tried is just having NOT_SET = object() and then setting that to each default. But that has a couple issues. (1) Mypy ignore goes crazy. (2) Super, duper annoying to subclass because of reason 1.

Anyway... tough problem 😬 I don't even know what to do, and even if I knew what to do, implementing it in a super user-friendly way is also a challenge.

Answer 2 · 2023-12-29T23:19:09.000Z

I'm thinking on settling on something like this (rough draft of documentation):

# Configuration

There are two methods to configure rich-click:

- Decorator: Use the `@rich_config()` decorator (and `RichHelpConfiguration()`).
- Globals: Set the global variables in the `rich_config.rich_config` module.

## Configuration using the `rich_config` decorator

Initializing a new `RichHelpConfiguration` object creates a configuration that you can then pass to your CLIs via the `rich_config` decorator. For example:

```python
import rich_click as click

@click.command()
@click.rich_config(help_config=click.RichHelpConfiguration(style_option="bold red"))
def cli():
    """Help text here."""

cli()
```

`RichHelpConfiguration()` initializes the default configuration, and the user is able to specify any changes to that default configuration that they'd like. Note that `RichHelpConfiguration()` is unaware of the global configuration.

You may also specify custom config as a dict:

```python
import rich_click as click
from rich_click import rich_config

@click.command()
@rich_config(help_config={"style_option": "bold red"})
def cli():
    """Help text here."""

cli()
```

There is a subtle difference between using a `dict` and using a `RichHelpConfiguration`. Initializing a `RichHelpConfiguration` creates a fresh config from the defaults, whereas a `dict` merges to either the parent or (if the parent config does not exist) the global config.

In the below example `subcommand`'s configuration would get "merged" into `my_group`'s configuration, meaning that `subcommand` would inherit the `style_option="bold red"` style from `my_group`:

```python
import rich_click as click
from rich_click import rich_config

@click.group()
@rich_config(help_config={"style_option": "bold red"})
def my_group():
    """Help text here."""

@my_group.command()
@rich_config(help_config={"style_argument": "bold yellow"})
def subcommand():
    """Help text here."""

cli()
```

## Configuration using the global config.

The other way to configure rich-click is to use the global configuration inside the `rich_click.rich_click` module:

```python
import rich_click as click
import rich_click.rich_click as rc

rc.STYLE_OPTION = "bold red"

@click.command()
def my_command():
    """Help text here."""

cli()
```

## Config resolution order (advanced)

It probably should not matter for most use cases, but just case it does matter, there is an explicitly defined order of operations for how the configuration gets resolved:

```mermaid
flowchart TD
    A["Did you pass in a @rich_config(help_config=...)?"]
    A --> |Yes| Ayes
    A --> |No| Ano

    Ayes["Was it a dict or a RichHelpConfiguration?"]

    Ayes --> |dict| AyesBdict
    Ayes --> |RichHelpConfiguration| AyesBrhc

    AyesBdict["Is there a 'parent' config?"]
    
    AyesBdict --> |Yes| AyesBdictCyes
    AyesBdict --> |No| AyesBdictCno

    AyesBdictCyes:::StoppingPoint
    AyesBdictCyes["Merge into the parent config,\nand use that"]

    AyesBdictCno:::StoppingPoint
    AyesBdictCno["Merge into the global config,\nand use that"]

    AyesBrhc:::StoppingPoint
    AyesBrhc["Use the RichHelpConfiguration  object.\n\n(Note: RichHelpConfiguration's\ndefaults are independent of the\nglobal config.)"]

    Ano["Is there a 'parent' config?"]

    Ano --> |Yes| AnoByes
    Ano --> |No| AnoBno

    AnoByes:::StoppingPoint
    AnoByes["Use the parent config"]

    AnoBno:::StoppingPoint
    AnoBno["Use the global config"]

    classDef StoppingPoint fill:#CC2200;
```

And here is that flow chart, by the way!

flowchart TD
    A["Did you pass in a @rich_config(help_config=...)?"]
    A --> |Yes| Ayes
    A --> |No| Ano

    Ayes["Was it a dict or a RichHelpConfiguration?"]

    Ayes --> |dict| AyesBdict
    Ayes --> |RichHelpConfiguration| AyesBrhc

    AyesBdict["Is there a 'parent' config?"]
    
    AyesBdict --> |Yes| AyesBdictCyes
    AyesBdict --> |No| AyesBdictCno

    AyesBdictCyes:::StoppingPoint
    AyesBdictCyes["Merge into the parent config,\nand use that"]

    AyesBdictCno:::StoppingPoint
    AyesBdictCno["Merge into the global config,\nand use that"]

    AyesBrhc:::StoppingPoint
    AyesBrhc["Use the RichHelpConfiguration  object.\n\n(Note: RichHelpConfiguration's\ndefaults are independent of the\nglobal config.)"]

    Ano["Is there a 'parent' config?"]

    Ano --> |Yes| AnoByes
    Ano --> |No| AnoBno

    AnoByes:::StoppingPoint
    AnoByes["Use the parent config"]

    AnoBno:::StoppingPoint
    AnoBno["Use the global config"]

    classDef StoppingPoint fill:#CC2200;

Answer 3 · 2024-04-10T03:11:43.000Z

I got to a good point with this and I'm quite happy.