search

tldr-pages / tldr-python-client

:snake: Python command-line client for tldr pages 📚
https://pypi.org/project/tldr/
MIT License
599 stars 95 forks source link

Use default terminal colors instead of white by default #96

Closed MasterOdin closed 4 years ago

MasterOdin commented 4 years ago

Closes #95

Closes #94 (sorta)

Removes the white default color from TLDR commands, as well as allows you to specify a blank string for one of the colors and it will work as expected by using the default color from the terminal.

This also fixes an issue where if you could only one word in the environment string and not a combination of things as the user_value not in ACCEPTED_COLORS check would always fail otherwise.

k4j8 commented 4 years ago

Works great for me, but do we want to set blank, name, description, and parameter to an empty string so that no user configuration is required?

zlatanvasovic commented 4 years ago

@KyleWJohnston It's compatible with the way colors are done in Bash. If you specify none, it's set to default.

k4j8 commented 4 years ago

Whoops, I was looking at the wrong branch. It works without configuration for me too.

zlatanvasovic commented 4 years ago

@MasterOdin @KyleWJohnston I think blank color can be dropped altogether, as it's supposed to be... blank. I'm working in #98 on simplifying the way that pages are displayed.

MasterOdin commented 4 years ago

I suspect the idea of the blank variable is that if you wanted to give a colored background for the entire output, you would want to set the background there. Like, in #20, if you just outputted an empty line, then the lines with no text would have a black background while the other lines would still have blue background here and there, and it would look like a mess.

zlatanvasovic commented 4 years ago

@MasterOdin Check the latest change. It should be highlighted perfectly now.

MasterOdin commented 4 years ago

@zdroid Given that this PR is necessary to get colors and attributes working as expected (and from environment variables), and have a meaningful discussion on BLANK (which you are removing / editing in #98), I would probably recommend merging this PR first, and then continuing the discussion on removing BLANK (or not) there.

zlatanvasovic commented 4 years ago

Fair point, I realized that too after making the changes.