Better color detection #111
Conversation
|
Oh and since this changes the CLI the completions for bash/zsh/fish should be updated as well. I can look into working on that, but I won't have time till at least a week from now and also haven't setup shell completions before if anyone more knowledgable can help. |
There was a problem hiding this comment.
Hi, thanks for the PR! From the description your work looks very thorough 🙂
I'll try to review the PR this weekend, but can't promise anything.
I can look into working on that, but I won't have time till at least a week from now and also haven't setup shell completions before if anyone more knowledgable can help.
"Best effort" is OK (adding the strings without testing), since with #108 we'll auto-generate those completions anyways.
These changes were all tested exclusively on Linux since I don't have a rust dev environment setup for Windows and I don't have access to any other systems.
CI runs on macOS and Windows as well. Maybe we can create integration tests for some of these cases?
test_markdown_rendering seems to be failing because of changes to the tar page
That was fixed in #110, I simply forgot to merge that one. Sorry about that.
--color always (The previous behavior of tealdeer)
Will use styling as long as ANSI support is detected.
I wonder if always should use ANSI support detection at all... That somehow seems to contradict the term "always".
|
Thinking back on it now it does seem deceptive to have a case where I should be able to find the time to:
over the next couple days. Just a bit busy with the end of the semester approaching after all |
CosmicHorrorDev
force-pushed
the
better-color-detection
branch
from
0189ab1
to
c6cdddd
Compare
|
So I found time to go ahead and make the changes. I based the shell completions off the |
CosmicHorrorDev
force-pushed
the
better-color-detection
branch
from
c6cdddd
to
19e2905
Compare
|
Saw this got some merge conflicts over time so I decided to go ahead and fix them. |
Great, thanks, and sorry for the delay! I applied rustfmt on the master branch, this should get rid of your first commit. There's also a conflict in |
|
I can fix it and rebase if you give me about a week (semester is busy atm). But if you don't mind doing it yourself and pushing the changes then feel free! |
This change makes the color for the "stale cache" warning follow `enable_styles`, like the rest of the styling.
dbrgn
force-pushed
the
better-color-detection
branch
from
19e2905
to
c840546
Compare
|
@LovecraftianHorror great, I went ahead and rebased your branch. I also squashed a few of the commits (e.g. the test fixes). Will try to review (and ideally merge) the PR tonight. |
dbrgn
force-pushed
the
better-color-detection
branch
from
c840546
to
dc6af5a
Compare
There was a problem hiding this comment.
Thanks @LovecraftianHorror, this looks great 🙂 I only fixed a small nitpick (didn't want to bother you with a change request), now it should be ready to merge if CI passes.
There's one more open PR, if I get that reviewed soon I should be able to create a new release next week.
Hello, first time hacking at this codebase.
So the aim of this pull request is to address the issues mentioned in #81. This was done by adding in a command line flag
--colorwith the optionsalways, auto, never.--color <WHEN>--color always(The previous behavior oftealdeer)Will use styling as long as ANSI support is detected.
--color autoWill use color unless:
NO_COLORenvironment variable exists as is the spec listed on theNO_COLORwebsite.stdout(e.g. when the output is being piped to another program liketldr man | xclip -sel clip). This is done with the help of the newly addedattylib since it promises cross platform detection for such cases.--color neverDisables colored output entirely.
Misc
check_cacheto make style checking consistent.test_markdown_renderingseems to be failing because of changes to thetarpage compared totar-markdown.expectedthat I left failing as it's outside the scope of this PR.