Improve command output with text banner, colour and alignment

[sadielbartholomew]

Close #105, including adding primitive colour to the terminal STDOUT, which can be disabled with a new CLI option --no-colour (shorthand -n) like with other standard commands, which make use of colour by default, allowing the choice to not add colour at all.

With this PR we go from a (human-readable, machine --format=json output unchanged with no colour) output without an ASCII text banner, to one with one before any logging or printing output, so from this:

$ cats --duration 480 --location "RG2"
WARNING:root:Unspecified carbon intensity forecast service, using carbonintensity.org.uk
Best job start time 2024-08-26 23:52:13.779577+01:00
Carbon intensity if job started now       = 150.64 gCO2eq/kWh
Carbon intensity at optimal time          = 150.64 gCO2eq/kWh

Use --format=json to get this in machine readable format

to this (add banner, align figures, subsume the format info message into the logging system):

$ cats --duration 480 --location "RG2"

The.____ ..... __ .... ________ . ______...
.. /  __)...../  \....(__    __).)  ____)....
..|  /......./    \......|  |...(  (___........
..| |limate./  ()  \ware.|  |ask.\___  \cheduler
..|  \__...|   __   |....|  |....____)  )....
...\    )..|  (..)  |....|  |...(      (..

WARNING:root:config file not found
WARNING:root:Unspecified carbon intensity forecast service, using carbonintensity.org.uk

Best job start time                       = 2024-08-27 10:14:59.692606+01:00
Carbon intensity if job started now       = 191.95 gCO2eq/kWh
Carbon intensity at optimal time          = 109.52 gCO2eq/kWh

though note I can't demonstrate the colours introduced through the a markdown code snippet, so here is a demonstrative screenshot of what output you get from the same command on the current main and on this branch:

Reviewing

I am happy to tweak any subjective elements e.g. the styling of the ASCII banner or the colours chosen, etc., but thought for progress it best put this PR up as a suggestion on these.

As for the banner design chosen for this starting point:

• I used the site linked in my Issue for the main lettering;
• took the 'Goofy' font option as my favourite choice;
• replaced whitespace in the background with dots which resemble plot gridlines to me and they make it somewhat customised and less generic (i..e not pulled as-is from the site I used);
• added to the dots at the end to form an arrow shape, to further customise it;
• added some colours on the lettering to make it stand out more.

I could go a little crazier with the colours but thought it best to keep it fairly minimal mostly to highlight any green as 'good' (or strictly, 'better') and red as 'bad' ('worse') for the figures as well as a little, but less so, emphasis on the CATS banner text.

[sadielbartholomew]

Looks like I need to update the tests - so moving this PR back to draft form. I'll do that this evening hopefully then re-open for review.

[abhidg]

Looks great! I tested on a light terminal and it works okay there as well. Windows might not work - but I think the main use case of CATS is on Linux machines anyway.

[sadielbartholomew]

Thanks @abhidg for the review. Very good point, I didn't think to check how the output looks on a light-on-dark (light background) colour scheme, since I always use light-on-dark. What you show in your screenshot looks fine in my view, too, though I think I should probably change the colour of the 'CATS' capital lettering to be the default colour rather than white (so black in your screenshot case, like the plain text), to ensure it is always visible.

In the next few days I shall get the tests updated here and add some testing of the new --no-colour option, then request a final review, but anyone who wants to (re-)review before or after the testing updates is welcome to.

[andreww]

Yep - looks good to me - I like it. Couple of questions.

First, what does this do for people who use screen readers in a terminal? I've no idea if people do use screen readers for working in a terminal (or, indeed, who to ask for advice on this) but I do wonder if an option to revert back to not using a text banner could help with accessibility. Actually, there seems to be one study (it claims to be the only one) at https://dl.acm.org/doi/fullHtml/10.1145/3411764.3445544. Section 5.4 may be relevant. Having said that, I suspect our json output format is already a big win for that. I'm agnostic about this.

Second, do you think it's worth putting the option to turn the banner and/or the colours off in the configuration file (or should we just expect that people know how shell aliases work)? Again, I'm agnostic.

[sadielbartholomew]

Thanks @andreww for the review. Very good point RE accessibility.

I do wonder if an option to revert back to not using a text banner could help with accessibility.

Given this thought I am wondering if we could perhaps replace the 'no colour' option I've used in this PR as-is with a more general 'minimal' (--minimal-view or similar) or 'plain' (--plain or similar) option which would both remove the colouring and the text banner? And, as a bonus, prevent us from having to wrestle with the American/British spelling of colour issue?

Second, do you think it's worth putting the option to turn the banner and/or the colours off in the configuration file (or should we just expect that people know how shell aliases work)? Again, I'm agnostic.

Good point also. I will look into the means to specify via the configuration file when I update this PR, hopefully tomorrow. Though since it concerns the command output itself, I personally think it is OK to have it just as something for the CLI, unless all of the other options are possible to set both from a config. file or the command-line, in which case I don't want to create some weird special case.

I'll report back once I update the PR.

[codecov]

Codecov Report

Attention: Patch coverage is 70.83333% with 7 lines in your changes missing coverage. Please review.

Project coverage is 88.48%. Comparing base (31cfede) to head (fc105a5).
Report is 22 commits behind head on main.

Files with missing lines	Patch %	Lines
cats/__init__.py	68.18%	7 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #113      +/-   ##
==========================================
- Coverage   89.23%   88.48%   -0.76%     
==========================================
  Files          14       14              
  Lines         548      573      +25     
==========================================
+ Hits          489      507      +18     
- Misses         59       66       +7     

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

[sadielbartholomew]

Hi @abhidg @andreww, sorry this fell off my radar but I've come back now and finalised it to include addressing your feedback. Changes since your initial review are:

• changing the colour of the 'CATS' outline lettering in the ASCII banner from 'white' to default to work better for dark-on-light terminal modes, since in the preview of Improve command output with text banner, colour and alignment #113 (comment) the white is visible as a grey but I worry it might not be with different colour schemes so better to have it as the default colour so it comes out dark on dark-on-light and light on light-on-dark;
• adding the no-color alias as per Andrew's good suggestion, to prevent the question of British/American spelling (ignore my first reply in Improve command output with text banner, colour and alignment #113 (comment) RE this, it is simplest to apply your suggestion directly);
• fix some typos I noticed along the way;
• and get the tests passing after making some fixes to issues in the logic.

One thing I haven't included is logic to add the --no-colour choice to the configuration file, since I've looked through the modules and I am not entirely sure where and how to implement that - does anything need specifying to include this new configuration option, or can it just be freely added to the YAML and accepted that way?

I haven't included explicit testing for the colour of the STDOUT since I don't know and can't, from some online investigation, see a way to test on whether the output is rendered in colour, but I guess I could test on having the right ANSI escape sequences in the output is good enough? If you agree, I will add a few tests that check that.

I've also realised there are some documentation snippets with the STDOUT shown which I will need to update in line with this PR, so there are a few more commits to add but in a self-contained way (tests, docs), so feel free to re-review my updates at this point, or wait until I've added those if you prefer.

[sadielbartholomew]

Bump @GreenScheduler/cats team - this PR has been sitting there a while now but is ready to go in my eyes - it has already had a few reviews and I have responded to all feedback after those so just needs a quick re-review to ensure the new commit changes are OK and/or an overall sanity check.

[abhidg]

Thanks, looks good!