michael-lazar / rtv

Browse Reddit from your terminal
MIT License
4.65k stars 274 forks source link

Support an rtv-specific PAGER setting #567

Closed jjmcdn closed 6 years ago

jjmcdn commented 6 years ago

This could just be a special-case of #541 where --pager=foo would set the value for PAGER for a given rtv instance, but given there's already RTV_BROWSER with precedence over BROWSER, RTV_EDITOR for EDITOR and RTV_URLVIEWER for URLVIEWER, it seems like RTV_PAGER would be a logical thing to set for a rtv-specific setting for PAGER.

jjmcdn commented 6 years ago

I have an implementation of this in my fork: https://github.com/joeythesaint/rtv/tree/jjm/rtv-pager that works reasonably well with my external md viewer and is easily testable with something like this:

$ export RTV_PAGER="gvim -" $ rtv

Then opening a submission or thread with 'l'.

michael-lazar commented 6 years ago

Out of curiosity, what's the reason that you want to set the rtv pager different from your default system pager?

jjmcdn commented 6 years ago

It's so that I can set RTV_PAGER to be something like mdless but I don't want to use mdless for all PAGER duties.

I'd have cited that in my test comment above, but the stock version of mdless exits when the page is fully rendered, causing the same problem you get with rtv you would get if you set PAGER=more instead of PAGER=less. I've made local patch to mdless to prevent that from happening, but I'm sure one of the other half-dozen command-line markdown viewers available implements proper less behaviour out of the box. That'll give rtv users a nicer overall experience when viewing long comments that include markdown, I think.

There are certainly other ways to accomplish this, and until I wrote up the patch over the weekend, I had been invoking rtv like:

$ PAGER=mdless rtf

Which keeps PAGER sane for most stuff and sets it to what I want for this particular command. I just threw this together after I'd run that from my command history again and thought it might be better to have a convenience variable.

michael-lazar commented 6 years ago

Thanks, that makes a lot of sense. I haven't heard of mdless before but it looks really cool. I would be fine with merging your branch if you want to make a PR!

jjmcdn commented 6 years ago

Okay, cool. PR #570 submitted. Thanks!

michael-lazar commented 6 years ago

Merged in https://github.com/michael-lazar/rtv/pull/570