Skip to content

Conversation

@MasterOdin
Copy link
Contributor

@MasterOdin MasterOdin commented Apr 29, 2020

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
Copy link

k4j8 commented Apr 30, 2020

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
Copy link
Contributor

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

@k4j8
Copy link

k4j8 commented Apr 30, 2020

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

@zlatanvasovic
Copy link
Contributor

@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
Copy link
Contributor Author

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
Copy link
Contributor

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

@MasterOdin
Copy link
Contributor Author

@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
Copy link
Contributor

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

@zlatanvasovic zlatanvasovic merged commit 9dde354 into tldr-pages:master Apr 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Do not specify default color by default for white text

3 participants