Respect $PAGER environment variable

[bptato]

Hi, thanks for writing this program, it's very useful.

This is a short patch I have used for a while to keep default $PAGER values even without the -p flag.

Normally, jiten would override it with less unless the user explicitly set the pager through -p. But I use a more capable pager than less, and having to set the pager for every jiten call is inconvenient.

Originally, I just ignored the "less -FR" default when $PAGER was set. But this is problematic from a backward-compatibility perspective: currently, the "-FR" options are added even if the user's $PAGER equals "less", as it is by default on many operating systems. On such systems you would suddenly get the less UI for single-page entries too, even though it just dumped those to standard out before.

So I made it set "less -FR" if $PAGER is missing or equals the string "less", which should be compatible with the old behavior.

[obfusk]

Thanks! I agree simply overwriting $PAGER isn't ideal. And I definitely want to support your use case of not having to use -p every time. But I'm not entirely sure this solution is ideal either as it's practically impossible to tell when $PAGER is set to something that should be overwritten to get the desired behaviour (like now) or when it should be used as-is.

I'll have to think about it for a bit. Would being able to set e.g. JITEN_PAGER instead of specifying -p work for you?

[bptato]

But I'm not entirely sure this solution is ideal either as it's practically impossible to tell when $PAGER is set to something that should be overwritten to get the desired behaviour (like now) or when it should be used as-is.

Good point. I think there's no perfect solution here, only trade-offs :/

My idealistic self would say "PAGER is meant to be used as-is", but practicality is more important than purism... hence the proposed (admittedly imperfect) heuristics. My thinking behind these is:

• If there is a system default for PAGER that the user didn't bother changing, it's probably "less". Even if they set it intentionally, adding these two flags is not nearly as unexpected as calling an entirely different program is.
  -> the existing behavior is unlikely to trouble less users, so let's keep it.
• Otherwise, the user most likely set PAGER intentionally. If the user sets something intentionally, they probably mean it - ignoring the sole signal of their preference is bad UX. (And it isn't even guaranteed to work, as less is not in POSIX.)
  -> non-"less" PAGER variables are best left unchanged.

Failure modes:

• less user wants pagination for single-page entries too (i.e. want less to be called without flags). This just means they have to keep passing -p, i.e. no change in the status quo.
• Non-less user wants to use less, but only for jiten. Sounds like an unusual use case, but at least now the standard method of PAGER=less jiten works just like in other programs.

Overall, this still looks like much better UX than requiring users to set a non-standard env var or a CLI flag, but I'm also biased by not using less :P

P.S. I've just realized: with the patch applied, jiten -p "" behaves incorrectly when PAGER=less. Will fix that in a second, apologies.

[obfusk]

I'll need to think about this a bit more. There's several interacting factors making this complicated.

• less -FR changes two settings: it enables ansi colour support and --quit-if-one-screen; other pagers may have different defaults for each of those.
• detection of colour support (which needs -R for less) in click only works for less (and checks both $PAGER and $LESS, setting the latter under some circumstances).
• what if you have $PAGER set but don't want to use it for jiten? I don't think having to use -p or PAGER=less jiten is ideal here; perhaps being able to override $PAGER specifically for jiten w/ $JITEN_PAGER like $GIT_PAGER makes sense.

As you said, "there's no perfect solution here, only trade-offs". And different users may have different expectations of what "the right thing to do" is here. So I want to take some time to try to find the best trade-off.

[bptato]

less -FR changes two settings: it enables ansi colour support and --quit-if-one-screen; other pagers may have different defaults for each of those.

-F seems less-specific; other pagers I know of either don't have it (w3m, most) or default to the equivalent behavior (more, vimpager).

-R is a bit more problematic, in particular when it comes to more: POSIX does not specify whether it should accept colors. e.g. Debian more displays colors, and OpenBSD more ("actually less(1) in disguise") cannot display colors at all. So it seems safest to not pass colors to more by default.

Other pagers I've tested all display colors by default, so no equivalent flag is needed.

detection of colour support (which needs -R for less) in click only works for less (and checks both $PAGER and $LESS, setting the latter under some circumstances).

Yeah, that's unfortunate :/ I guess an upstream patch will be needed to add a list of color-capable pagers other than less.

A JITEN_COLOR env var may also be useful.

(Now I dream of a standard COLORPAGER env var, similar to COLORTERM; alas, it's not a thing.)

what if you have $PAGER set but don't want to use it for jiten? I don't think having to use -p or PAGER=less jiten is ideal here; perhaps being able to override $PAGER specifically for jiten w/ $JITEN_PAGER like $GIT_PAGER makes sense.

Sounds reasonable. An alternative would be a config file, but that's kind of excessive for just two options :P

[obfusk]

Thanks for confirming stuff about other pagers. Only thing I'm not sure about is how best to balance backwards-compatibility with "doing the right thing" (I agree it should do its best to respect $PAGER) since I don't know how many other users would benefit from either.

[bptato]

Only thing I'm not sure about is how best to balance backwards-compatibility with "doing the right thing"

Well, that just depends on whether you see ignoring PAGER as a bug or a feature :P

In my view, it's a bug: PAGER, EDITOR, etc. are well established standard variables that indicate clear intent from the user. So instead of aiming for bug for bug compatibility, I think the best solution is to fix PAGER != less and stay backwards-compatible if PAGER == less (as in this patch).

I do see that the Click coloring issue makes doing this right now a bit problematic ("ok, you get your PAGER, but no more colors!" :P), I will submit a patch to upstream to fix this.

[obfusk]

I think we agree. I meant "doing the right thing" = respect $PAGER :)

But indeed if we do that, some users might lose colours or otherwise get behaviour that would now be correct but result in changes in functionality for them. And that is something I only want to do if it's worth it. But I'm not sure how to weigh those options against each other since I have no information on how many users would be affected and, if so, how.

If not for existing users that may (unknowingly) be relying on the incorrect behaviour we would not be having a discussion here :)

[obfusk]

Sadly, I made the mistake of not respecting $PAGER when originally implementing this and fixing that mistake cannot be done in a way that is guaranteed to be backwards-compatible for all users.

[bptato]

I agree with your points.

I am happy to report that I have found a better way to integrate Jiten with my pager in the meantime, so I am no longer affected by this issue. Accordingly, I will be closing this ticket.

Thank you for your consideration, and sorry for bothering you.

[obfusk]

I am happy to report that I have found a better way to integrate Jiten with my pager

I am happy to hear that :)

sorry for bothering you

Not at all. Thank you for bringing the issue to my attention and providing various details and insight. I'm still unsure what the best fix is, but I do think some improvement can be made here. I'm glad you found a solution that works for you and are thus not affected by the time it will take me to figure that out.