Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Colour code text output for clarity #1895

Closed
darrelmiller opened this issue Oct 16, 2022 · 3 comments · Fixed by #1905
Closed

Colour code text output for clarity #1895

darrelmiller opened this issue Oct 16, 2022 · 3 comments · Fixed by #1905
Assignees
Labels
enhancement New feature or request fixed generator Issues or improvements relater to generation capabilities.

Comments

@darrelmiller
Copy link
Member

The output from commands, while helpful, can be challenging to read in some cases.
image

Could we consider colour coding different types of response output?

@baywet
Copy link
Member

baywet commented Oct 16, 2022

We can, but I need some details. Error red, success white, hint blue? Some of them are already used by the logger.
What should happen if the terminal has a fancy color scheme?

@baywet baywet added Needs: Author Feedback enhancement New feature or request generator Issues or improvements relater to generation capabilities. labels Oct 16, 2022
@baywet baywet moved this to Todo in Kiota Oct 16, 2022
@baywet baywet added this to Kiota Oct 16, 2022
@baywet
Copy link
Member

baywet commented Oct 17, 2022

From a quick search there are a couple of considerations to think of went playing with colours in console apps (asides from accessibility).

Most modern terminals support AINSI colors, but not all (windows cmd host didn't support it until a couple of years ago).
https://www.jerriepelser.com/blog/using-ansi-color-codes-in-net-console-apps/

Regardless of the colours we pick, the terminal might/should remap them to a visible colour with its theme. It's not the job of the console app (in fact we don't have access to the terminal theming information)
https://www.ubuntupit.com/best-windows-terminal-themes-and-color-schemes/

e.g. if we decide to write something in black and the background is black, the terminal should map black characters to something like a grey or another colour so the user can see what's being written.

With that in mind, we also need to consider we have logging in place with ILogger, and maybe one day tracing if we implement #1486 on top of Console.Write.
Tracing writes to the terminal in white as far as I can remember from my observability work if a synk to the console is setup.
Logging uses multiple colours, and is enabled by default:

  • Red for errors
  • Yellow for warning
  • white for anything above warning (remapped to blue in the debug console of VS code for example)

My suggestion is to keep that colour scheme for the logger and use the following scheme for Console.Write messages:

  • blue for hints
  • yellow for warnings (e.g. no matches found with search)
  • red for errors, although those will most often be written by the logger
  • green for successful status (e.g. generation completed, download done, etc...)
  • white for details (search table, info result, search details)

Thoughts?

@darrelmiller
Copy link
Member Author

Yes. Completely agree with the colour suggestions.

@darrelmiller darrelmiller assigned baywet and unassigned darrelmiller Oct 18, 2022
baywet added a commit that referenced this issue Oct 18, 2022
@baywet baywet linked a pull request Oct 18, 2022 that will close this issue
@baywet baywet added the fixed label Oct 18, 2022
@baywet baywet moved this from Todo to In Progress in Kiota Oct 18, 2022
baywet added a commit that referenced this issue Oct 18, 2022
- fixes #1895 adds a colours scheme for console messages
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request fixed generator Issues or improvements relater to generation capabilities.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants