Feature/improve colour contrast

[Voultapher]

What

• Change default colour scheme to high contrast using terminal default colour
• Highlight program name in code examples

Before:

After:

Why

Right now the only reason I use a painfully slow python version of tldr is because its colour scheme makes quickly glancing at a tldr and seeing what I'm looking for much easier than when looking at the monotone low contrast produced by the current version of tealdeer.
The current contrast of most likely dark background to teal is challenging for certain people with colour blindness. I understand that the project is called tealdear and removing the teal colour might make the pun obsolete.
If you prefer the old colour scheme, I suggest making it an option. People that want another scheme could do alias tldr=tldr --color=teal.

Implementation Notes

I chose to go with string indexing over split and flat_map as I wanted to avoid a performance regression. In a micro benchmark, format_code is faster and more consistent than format_braces whilst doing more work. Yet it is seems to be only a small fraction of the overall time, which means I wouldn't be opposed to rewriting it in a more readable fashion.
Pulling the command from the title isn't pretty, the command name is present when called regularly, however when using the option --render we would have to either introduce a new parameter to specify the command, which would increase usage burden and could introduce user errors, or accept different behavior for regular use vs --render which I find particular bad. This solution doesn't change the current interface and behaves the same way in regular use and --render. However I have not tested if all tldr files correctly specify the program title in their title.

[Voultapher]

Also if this is accepted we should update the README example picture.

[dbrgn]

Hi. Thanks for your pull request!

I understand that the project is called tealdear and removing the teal colour might make the pun obsolete.

That was actually not intentional, I never noticed 🙂

Regarding the proposed change, first of all, do you use some customizations regarding your terminal colors? In a regular dark-background xterm session, the output looks like this:

Quite high-contrast to me, I'm not sure why it looks so uniform on your terminal.

In contrast, your version looks like this:

To me that's less easy to parse (except that highlighting the program name is a great idea).

[Voultapher]

do you use some customizations regarding your terminal colors

Yes. I use konsole with a custom color scheme.

I think when it comes to color schemes, we can't make one size fits all. The best we can do is something that is good in most cases. To get a better understanding I tried out all standard konsole color schemes with the new highlighting.

Most of them are fine. The explicit white for the description creates problems in the caption for some of the bright schemes. My understanding is that we want a clear contrast between the example text and the example code. Coming back to not being able to make it perfect for everyone, I believe the default color is likely to always be clearly readable. If we stick to giving either the example text or example code another color, we greatly increase the risk of making the 2 styles incompatible. By highlighting the program name in example code, I believe there is a noticeable difference between example text and code. Red is often high contrast to the default color, and even when it is not only the program name is hard to read and the rest of the information which is likely more unknown is still legible.
I'd like the description to stand out, yet sticking to what I said about adding non default color increasing the potential for poor contrast, I suggest giving it the default color and somehow highlighting it differently.
2 examples

[Voultapher]

@dbrgn this has been without activity for 2 weeks now. How should we proceed?

[dbrgn]

@Voultapher sorry, I already typed a reply but never sent it off. I'm a bit tight on free time at the moment.

I think the best solution in this case would be theme support (in the sense of a config file where you can specify colors from an ANSI color palette), so that people with non-standard terminal colors can customize the output themselves. However, it should not impact performance in case the defaults are used.

Right now we don't have any configfile support. Maybe checking for a configfile (using dirs or directories) when starting up would be an option. If no config file exists, just carry on. If a config file exists, parse it (I think I'd prefer toml format that's being parsed into structs) and apply a theme if it exists.

[Voultapher]

Ok, will look into it.

[dbrgn]

@Voultapher hehe, actually last week I switched to alacritty by default (instead of uxterm) which uses a slightly different theme with less contrast between green and yellow, for example, so I'm more sympathetic to your feature request now 😄

[Voultapher]

Ok, syntax config support and tests for it are done, concerning performance implication of this feature:

brach name:

time in seconds
instruction count

master:

0.010360291
19,279,851

0.012931068
17,382,742

0.011129954
12,447,344

0.009349510
17,045,595

0.009982335
18,797,206

feature/improve-colour-contrast | no config file:

0.011297944
17,443,583

0.011954686
18,474,816

0.011131505
18,740,495

0.010944560
16,565,537

0.010744000
17,389,713

feature/improve-colour-contrast | with config file:

0.010893010
18,202,644

0.009908770
15,949,160

0.010217479
16,634,313

0.010714000
18,750,531

0.011990885
14,365,538

This should not be a noticeable slowdown, no matter if a syntax config is used or not. Btw, serde is dope! Imo error categories, propagation and handling are not perfect, still I'm happy with the current state. Take a look and tell me if there is something to improve.

[dbrgn]

Very nice to see these performance numbers 👍 Serde is blazing fast.

I'll try to have a look next week if possible, I'll keep this tab open. If I forget for some reason, feel free to remind me in the last week of August, when I have a few free days 🙂

[Voultapher]

Friendly ping.

[dbrgn]

Friendly ping.

Thanks! Yes, it's still on my todo list for this week 🙂 The weather early this week was great, so I spent it paragliding instead of sitting in front of my computer 😉

[dbrgn]

Hi. Thanks for your contribution, nice PR! I did a first review pass, sorry for the many comments.

Unfortunately the output seems a bit broken:

The first words of the description text is highlighted too. Furthermore, the commands are duplicated tartar instead of tar. I do like tartar, but this should be fixed 😛 (Btw, you also copied that error into the test expectations.)

[Voultapher]

The first words of the description text is highlighted too. Furthermore, the commands are duplicated tartar instead of tar. I do like tartar, but this should be fixed stuck_out_tongue (Btw, you also copied that error into the test expectations.)

Wow, no idea how I missed that 😛

[Voultapher]

From my point of view there are only 2 outstanding items, multipurpose config files and RawConfig default initialization, both waiting for your feedback. Is there something else unresolved?

[dbrgn]

Good approach, with the helper function! 👍

[dbrgn]

Thanks @Voultapher for the changes after my last review!

I left some further comments. I think after those are handled, it should be ready to merge.

Once it's in the code, we can start thinking whether we want to optimize the default style, and how to share configs for different "themes".

[Voultapher]

@dbrgn only the description highlighting left unresolved. Everything else looks good.Also should we warn the user if tldr --seed-config is being run and a file is already present in that location?

[dbrgn]

Thanks for the adjustments!

Also should we warn the user if tldr --seed-config is being run and a file is already present in that location?

That's a good idea. I think even showing an error message à la "A configuration file already exists at , no action was taken" with a non-0 exit code would be fine.

Everything else looks good.

What do you think about the YAML proposal above? Shouldn't be too hard to try thanks to libraries like serde-yaml, and I think it would improve readability. (I'd be fine wtih TOML too though.)

[Voultapher]

What do you think about the YAML proposal above? Shouldn't be too hard to try thanks to libraries like serde-yaml, and I think it would improve readability. (I'd be fine wtih TOML too though.)

I'll repost what I wrote earlier about toml vs serde, it seems you didn't see it:

Personally prefer toml over yaml for confgs like this, also if we make the style config a member of the config object, there is no need to introduce a block. Looks like this:

[style.highlight]
foreground = "blue"
underline = false
bold = true

[style.description]
underline = false
bold = false

[style.example_text]
underline = false
bold = false

[style.example_code]
underline = false
bold = false

[style.example_variable]
underline = true
bold = false

To add to it. The yaml version would look something like this:

style:

  highlight:
    foreground: blue
    underline: false
    bold: true

  description:
    underline: false
    bold: false

  example_text:
    underline: false
    bold: false

  example_code:
    underline: false
    bold: false

  example_variable:
    underline: true
    bold: false

Not sure that is better.

[Voultapher]

@dbrgn from my side everything is resolved.

[Voultapher]

Friendly ping.

[dbrgn]

OK let's keep TOML for now :)

I'll do some more adjustments (like making the default style like the previous default) and will then merge the branch!

Thanks a lot @Voultapher for your patience :)

[dbrgn]

Merged in 278a2bb!

[dbrgn]

@Voultapher Ok, all open pull requests have been merged. I'll leave for 2 weeks of vacation tomorrow, some testing would be welcome during that time. If everything works, I plan to make the next release early October.