CLI: Add color flag to verdi process list

[agoscinski]

Adds the flag --color/--no-color to verdi process list that colors the process state if on (default is on). Implements feature request #4955.

In principle it is not hard to make the color and symbols configurable because we could pass the config from the context ctx.obj.config. To not pollute the config with a bunch of styling variables, I would only use one variable that is referring to a separate config file with all styling variables. EDIT: (But I think there are more important things to work on for now, but I would make a new issue about it.)

I thought about doing the option like the ls command --color=always|auto|never, it adds the option to enforce the color also when piping, so in ls -l > out the out file would have the color strings. But I never have found this option very understandable. Right now it works like --color = auto and --no-color = never.

I am actually not sure how to test this meaningfully.

EDIT:
the color scheme is like in the issue and also agrees with the color scheme in the plumpy doc https://plumpy.readthedocs.io/en/latest/concepts.html#process (I gu

I use click for styling since we use it for all CLI.

[agoscinski]

I did not like that this can return a None. I checked this function is no where used outside this file. If one wants the old behavior, then at least I would adapt the typehints

[sphuber]

We actually need this because process_state can be None. My database has some occurrences. I agree with updating the type hints though, so please do that.

[giovannipizzi]

Comment: it's not returning a None, but the string "None", right? so no need to fix type hints?

[sphuber]

Yeah, but the argument process_state to the function can be None so it should be

def format_state(process_state: str | None, ...) -> str:

[agoscinski]

there was the option to put the coloring logic into format_process_state, but since this part already handles the symbol logic, it made more sense to me to put it here for readability.

[agoscinski]

I had the option to add the coloring argument to CalculationQueryBuilder, but I felt that creates a nested entangled logic that is harder to understand than this.

[sphuber]

Still, the current solution is also quite weird and requires passing the coloring argument through to many steps. I see that it is tricky because it somehow needs to end up in the format_state function. Ideally the formatting is left to the actual command that prints the output, but that would have to be in aiida.cmdline.commands.cmd_process and that would also be quite ugly I am afraid.

Another thing I don't really like is that now aiida.tools imports click which is a command line utility. Normally, that should be kept to aiida.cmdline. But since it is just using the style method, it is not that bad. I wonder what these ansi codes render like in something that is not a terminal, e.g., a Jupyter notebook. Does it render properly?

With that all in mind. I wonder if there is a better solution. What if we just have format_state always add the color through ANSI escape codes. Although click does not (yet) support the NO_COLOR environment variable there is an (undocumented) property of the context color that can be set to False to disable all ANSI color codes. So what you could do in cmd_process is simply

def command(ctx, ..., color):
    ctx.color = color

And that's it. If the flag is turned off, all coloring is removed. This would allow to get rid off all the ugly passing through of color.

It would also make it easy to implement support for the NO_COLOR env variable as was mentioned by @danielhollas . What do you think?

[agoscinski]

Do I understand you correctly that we temporary disable it globally to get the formatting without colors, so we set it back to the original value afterwards so the echo_reports are printed with color?

def command(ctx, ..., color):
    ...
    color_before = ctx.color
    ctx.color = color
    projected = builder.get_projected(query_set, projections=project) # gets string to print out
    ctx.color = color_before
    ...

I don't know, it sounds also not logically clean to me to change a global state in a command to obtain some local changes down the line. I see your point that passing the coloring argument on object construction is clunky and it would be nicer do pass it with get_projection. This is not possible without more severe changes as the formatting as defined on initialization.

Ideally the formatting is left to the actual command that prints the output, but that would have to be in aiida.cmdline.commands.cmd_process and that would also be quite ugly I am afraid

I dont think doing it this way would be ugly, since we can iterate trough the column process state by projected and with map it should not introduce any severe performance issues. I will try this out, since this seems to me the nicest solution from all available ones.

It would also make it easy to implement support for the NO_COLOR env variable as was mentioned by @danielhollas . What do you think

I think this one is independent of this question, since the effect of this variables is global so it makes sense to me to implement them in src/aiida/__main__.py as suggested in pallets/click#2207 (comment)
I just implemented it like this

[sphuber]

I think this one is independent of this question, since the effect of this variables is global so it makes sense to me to implement them in src/aiida/main.py as suggested in pallets/click#2207 (comment)
I just implemented it like this

Except I don't think this will always work. It will work when you invoke the CLI through python -m aiida which will route through src/aiida/__main__.py. However, it won't work when you call verdi directly. That is exposed as an entrypoint and directly calls src/aiida/cmdline/commands/cmd_verdi.py::verdi.

Do I understand you correctly that we temporary disable it globally to get the formatting without colors, so we set it back to the original value afterwards so the echo_reports are printed with color?

It is a good question whether we even want to make a distinction here. If we provide a color option to a command, should it apply to all output? I actually think that makes sense. So instead of adding a one-off option to verdi process list here, maybe we should think to have --color be a global option that applies to all commands (just like --verbosity). When specified, it will switch of all coloring. To me this doesn't only make more sense, but it will also be easier and cleaner to implement. Instead of having to pass through color arguments throughout the implementation, it can now be handled in one place and just configures the click.Context so it can automatically handle the formatting and strip color codes.

[agoscinski]

Thanks for the clarification! I think implementing it like you suggest it as flag that is available for every command makes to me the most sense. Some commands are slightly problematic like verdi run that cannot enforce the coloring to its subprocess (or at least it needs extra logic). But I think it is acceptable in such cases (I think verdi restapi would be another) that the --no-color option does not work properly for now. Almost all commands print something to the terminal that is colorized by the echo utils. Maybe I can adapt the help page for these specific commands (verdi run and verdi restapi), so it is clear that it does not work properly.

[danielhollas]

Exciting! 🌈

I thoutght about doing the option like the ls command --color=always|auto|never, it adds the option to enforce the color also when piping, so in ls -l > out the out file would have the color strings. But I never have found this option very understandable. Right now it works like --color = auto and --no-color = never.

I feel quite strongly that the default behaviour should be auto i.e. you should somehow detect whether we're in tty mode, otherwise this new default behaviour will break users who pipe its stdout into another command.

Ideally, we'd also respect FORCE_COLOR and NO_COLOR environment variables.
https://force-color.org/
https://no-color.org/

(this is e.g. how Python 3.13 will handle colors in tracebacks as well)

[sphuber]

We actually need this because process_state can be None. My database has some occurrences. I agree with updating the type hints though, so please do that.

[sphuber]

Still, the current solution is also quite weird and requires passing the coloring argument through to many steps. I see that it is tricky because it somehow needs to end up in the format_state function. Ideally the formatting is left to the actual command that prints the output, but that would have to be in aiida.cmdline.commands.cmd_process and that would also be quite ugly I am afraid.

Another thing I don't really like is that now aiida.tools imports click which is a command line utility. Normally, that should be kept to aiida.cmdline. But since it is just using the style method, it is not that bad. I wonder what these ansi codes render like in something that is not a terminal, e.g., a Jupyter notebook. Does it render properly?

With that all in mind. I wonder if there is a better solution. What if we just have format_state always add the color through ANSI escape codes. Although click does not (yet) support the NO_COLOR environment variable there is an (undocumented) property of the context color that can be set to False to disable all ANSI color codes. So what you could do in cmd_process is simply

def command(ctx, ..., color):
    ctx.color = color

And that's it. If the flag is turned off, all coloring is removed. This would allow to get rid off all the ugly passing through of color.

It would also make it easy to implement support for the NO_COLOR env variable as was mentioned by @danielhollas . What do you think?

[codecov]

Codecov Report

Attention: Patch coverage is 63.33333% with 11 lines in your changes missing coverage. Please review.

Project coverage is 77.82%. Comparing base (ef60b66) to head (7187813).
Report is 39 commits behind head on main.

Files	Patch %	Lines
src/aiida/__main__.py	0.00%	9 Missing ⚠️
src/aiida/tools/query/formatting.py	88.24%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6434      +/-   ##
==========================================
+ Coverage   77.51%   77.82%   +0.32%     
==========================================
  Files         560      561       +1     
  Lines       41444    41790     +346     
==========================================
+ Hits        32120    32519     +399     
+ Misses       9324     9271      -53     

Flag	Coverage Δ
presto	73.14% <63.34%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[agoscinski]

I put the logic for the color and symbols of process states into aiida/common, but I am not sure maybe aiida/cmdline is a better place for it as it only effects the styling for the cmdline? But technically src/aiida/tools/query/formatting.py can be also used outside of the CLI.