Improve page display to match the screenshot

[zlatanvasovic]

Closes #79.

Summary of the changes:

• page format greatly improved
• text properly colored and highlighted
• blank color removed
• cprint replaced with colored
• get terminal size functions removed

Screenshot:

[zlatanvasovic]

@MasterOdin What do you think, should this be merged before #96? I want to avoid fixing tests twice at any cost.

[MasterOdin]

I'm fine updating my PR after this gets merged.

[zlatanvasovic]

ping @waldyrious

[zlatanvasovic]

@felixonmars Why was Manually adding painted in blank spaces added in the first place? It just slows down the program and also adds unnecessary whitespace on copy/paste.

I removed it in this PR.

[felixonmars]

git blame shows that it was added in https://github.com/tldr-pages/tldr-python-client/pull/20

[zlatanvasovic]

Pretty much perfect now.

Maybe it's even better with 2 indents for title and description?

[MasterOdin]

It feels like you're missing the point of the BLANK color, and removing it is not right, as it would prevent the styling of background for an entire sheet if so desired (like in the screenshot from #20).

command: TLDR_COLOR_BLANK="white on_blue" TLDR_COLOR_NAME="white on_blue" TLDR_COLOR_DESCRIPTION="white on_blue" TLDR_COLOR_EXAMPLE="green on_blue" python3 tldr.py cd

Before:

After:

Note, this does require my fix from #96 or commenting out lines 223-225 to get the color + attribute to work properly on your branch.

[zlatanvasovic]

Compare it to the way git (or pretty much any standard console tool) does it (all is highlighted, for the record, as my terminal background is white): Only colored parts are supposed to be selected. Adding spaces and blank backgrounds complicates the display and copy/paste.

What you seem to miss here is that your PR solves the problem you described: By setting the colors to default, highlighting matches perfectly.

[zlatanvasovic]

@MasterOdin For comparision, this is how it looks after merging your PR:

[MasterOdin]

I would say set TLDR_COLOR_BLANK to the empty string ("") by default, and that would fix your problem?

I mean, feel free to remove TLDR_COLOR_BLANK, but the idea of setting a background was something someone wanted when they added it.

Only colored parts are supposed to be selected. Adding spaces and blank backgrounds complicates the display and copy/paste.

I can buy complicating writing the display logic, but not sure I buy copy/paste. Testing out master in a terminal window on cmd.exe, macOS iTerm2 using zsh, and Ubuntu bash, when I selected and copied the entire page, there was no extraneous spaces on each line, regardless of background color or not. I suppose you could just skip printing out the content if TLDR_COLOR_BLANK is empty if it really messes up copy/paste for you, but it does sound like a bug in your terminal?

[zlatanvasovic]

@MasterOdin This is what I get in VSCode after copying and pasting the output from python3 tldr.py tldr. First screenshot is using Copy, the second is using Copy as HTML.

1. master

2. newline

(Note: it may seem there is trailing whitespace at the end of all lines, but that's not the case, just the peculiarity of VSCode's highlighting)

Bonus: HTML display also confirms that formatting on newline is correct (as it is on master):

---

To conclude, with all the changes I proposed (proper highlighting and removing the whitespace), blank as a color seems redundant completely.

I also don't see the reason to set tldr's background to something special (as basically no other command-line tool does that). The biggest problem I see with setting the background is that it requires adding whitespace. Such approach also bugs multi-line display of a single line, which is very common on 80x24 terminals.

[zlatanvasovic]

Note: lines 37-74 can be removed since columns and rows size of the terminal is not used anymore. Rows were actually never used, while columns were used only to add spaces.

[MasterOdin]

The background stuff was done in #1 which I guess was because the node client did at the time as well, and has just been carried forward since then. I cannot say whether or not people have been using this option, just that it has existed in tldr for 6 years at this point.

I also don't see the reason to set tldr's background to something special (as basically no other command-line tool does that).

That's the trick right? It's hard to say what end users are doing. While I also don't personally bother setting the background color, that's not to say there's someone out there who isn't. Given that it was added as a set hard-coded option to be a on_blue background, it's clear there's at least one person who thought that was the ideal way to display the pages in a terminal.

[zlatanvasovic]

I just checked tldr-node-client and it doesn't support custom background. This is how node client highlights the text (same as I proposed here, minus the correct highlighting for the - Example descriptions): (it also doesn't add any trailing whitespace...)

While I'm all in for adding cool features, if they bring more harm than good, I would rather keep them unavailable. Having a custom background for tldr is less worth than having a proper display and copy/paste. After all, users can set their own terminal background to anything they wish.

[MasterOdin]

I think it's more that the node client used to set the background, and the background stuff was added to the python client to make it feature parity with the node client. Can always just remove it here and see if anyone complains and re-address this debate then.

[zlatanvasovic]

@MasterOdin Can you come to Gitter for a moment?