search

tldr-pages / tldr-c-client

C command-line client for tldr pages
MIT License
293 stars 50 forks source link

Added an option to disable color highlighting #107

Closed Vedaant-Rajoo closed 9 months ago

Vedaant-Rajoo commented 9 months ago

What does it do?

Disable color display when the output is not a tty.

Why the change?

Color gives weird ANSI polluted output for non tty screens. tldr is used not only in terminals but other places like code editors like vim and emacs where this behaviour might not be simplified.

How can this be tested?

tldr tldr > tldr.md tldr --color tldr > tldr.md tldr tldr

Where to start code review?

parser.c

Relevant tickets?

closes #35

Questions?

<Ask us anything!>

SethFalco commented 9 months ago

Perhaps it's worth supporting NO_COLOR too while we're here? Or I could do it as a separate PR to streamline this one.

NO_COLOR is an informal standard/convention to disable color in command-line application output. It's usually implemented in color libraries, but since we don't use one we'd be responsible for doing it directly.

If NO_COLOR is set to any non-empty string, all colors are disabled.

kbdharun commented 9 months ago

Perhaps it's worth supporting NO_COLOR too while we're here? Or I could do it as a separate PR to streamline this one.

NO_COLOR is an informal standard/convention to disable color in command-line application output. It's usually implemented in color libraries, but since we don't use one we'd be responsible for doing it directly.

If NO_COLOR is set to any non-empty string, all colors are disabled.

Agreed, feel free to modify the PR.

SethFalco commented 9 months ago

I've added a commit, feedback welcome.

SethFalco commented 9 months ago

An issue I noticed with the branch though, it seems ./tldr -C tldr doesn't work though?

./tldr: unrecognized option '-C'

The long option works flawlessly, but before adding my commit, the short option wasn't working, I did not resolve this. 🤔

kbdharun commented 9 months ago

n issue I noticed with the branch though, it seems ./tldr -C tldr doesn't work though?

./tldr: unrecognized option '-C'

The long option works flawlessly, but before adding my commit, the short option wasn't working, I did not resolve this. 🤔

Interesting, I will try replicating this.

kbdharun commented 9 months ago

Update: Tested it right now, the force colour option works fine for me before your commit too.

I think this PR is GTG will merge it and focus on the other PR updating the man page.