Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve bfs -help with LESS already set #71

Closed
tavianator opened this issue Apr 14, 2021 · 2 comments · Fixed by #72
Closed

Improve bfs -help with LESS already set #71

tavianator opened this issue Apr 14, 2021 · 2 comments · Fixed by #72

Comments

@tavianator
Copy link
Owner

RE: @markus-oberhumer's #68

I think it makes sense to not override LESS, in case it's necessary for less to work properly in the user's environment. But I can think of a couple improvements that would make sense:

  • Override LESS if it's empty. Similar to the new PAGER handling. I think git ought to do the same thing.
  • If LESS is nonempty, don't colorize the help. This should at least make it readable for you. And if you want the colors, you could do LESS= bfs -help.

But I just thought of a complication: if $PAGER != less, it's weird for $LESS to affect the behaviour of bfs help. Thoughts?

@markus-oberhumer
Copy link
Contributor

I think it makes sense to not override LESS, in case it's necessary for less to work properly in the user's environment. But I can think of a couple improvements that would make sense:

* Override `LESS` if it's empty.  Similar to the new `PAGER` handling.  I think `git` ought to do the same thing.

Yes, that makes sense. BTW, my code from #68 is buggy as mixes putenv with the explicit envp parameter - I can submit a new version if you want.

* If `LESS` is nonempty, don't colorize the help.  This should at least make it readable for you.  And if you want the colors, you could do `LESS= bfs -help`.

But I just thought of a complication: if $PAGER != less, it's weird for $LESS to affect the behaviour of bfs help. Thoughts?

After some more googling around I wouldn't touch that code - more works out of the box, and it's probably time for me to change my defaults to export LESS="-R -MM -i".

Thanks!

@tavianator
Copy link
Owner Author

Yes, that makes sense. BTW, my code from #68 is buggy as mixes putenv with the explicit envp parameter - I can submit a new version if you want.

Oh true. Better to just change how parse.c constructs the new environment I think. bfs_spawn_addputenv() has some other semantic issues like how it interacts with PATH (come to think of it, I think my spawn.c is already wrong about this on some platforms), and it would make #47 even more challenging.

So if you feel like submitting a patch that just fixes the empty LESS case, I'd take it and consider this issue closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants