Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 513470 - app-admin/eselect: Use $PAGER on eselect news read
Summary: app-admin/eselect: Use $PAGER on eselect news read
Status: CONFIRMED
Alias: None
Product: Gentoo Hosted Projects
Classification: Unclassified
Component: eselect (show other bugs)
Hardware: All Linux
: Normal enhancement (vote)
Assignee: Gentoo eselect Team
URL:
Whiteboard: eselect-1.5
Keywords: NeedPatch
Depends on:
Blocks:
 
Reported: 2014-06-16 12:39 UTC by Sebastián Magrí
Modified: 2020-04-10 05:44 UTC (History)
2 users (show)

See Also:
Package list:
Runtime testing required: ---


Attachments
pager support patch (pager.patch,1.84 KB, patch)
2017-02-16 01:41 UTC, Teika kazura
Details | Diff
Patch to support PAGER for "eselect news read". (0001-eselect-1.4.16.patch,1.92 KB, patch)
2020-04-09 02:26 UTC, Teika kazura
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sebastián Magrí 2014-06-16 12:39:54 UTC
While normally there will be one or two new news items, on new installs or when you have long time without syncing, it will be difficult to read news on some terminals. Using $PAGER may fix this.

Reproducible: Always

Steps to Reproduce:
1. eselect news read

Actual Results:  
News are shown in the stdout without a pager

Expected Results:  
Pipe the news to a pager
Comment 1 Ulrich Müller gentoo-dev 2014-06-16 15:55:04 UTC
We should go with a solution similar to what is implemented in man or git.

Open questions:
- Git sets LESS and LV variables. Should we also do this?
- Presumably we need a --pager option that can override the environment.
  (man has a -P option; git has -p/--paginate/--no-pager options.)
- Not sure yet what to do with colour output. Not all pagers will accept it,
  therefore disable colour when output is through a pager? (Git passes colour
  escape codes to less, though.)
- News module only, or global feature?
Comment 2 Sebastián Magrí 2014-06-18 03:28:27 UTC
Regarding the first and fourth questions I believe global pager support might be useful. Having a global variable modules could use to enable paginated output when it makes sense sounds fine. For the second question, having the possibility to pass pager options would be great also.

About colored output, we might do what our default pager supports better.

Best Regards,
Comment 3 Teika kazura 2017-02-16 01:41:28 UTC
Created attachment 463898 [details, diff]
pager support patch

The attached file implements a basic pager support for "eselect news read".

When the environment variable PAGER is set, it is used. When stdin or stderr[1] is not attached to a terminal / console, then the pager won't be used.

A pager is used only to display each news item. So messages like "No news is good news." are shown without a pager. I think this is sensible.

I'm not sure about the questions in Comment #1. At the very least it should be possible to disable the pager by a command line argument, but I didn't implement it. (For the record:I posted my code at the Gentoo Forum on 5 Feb, and there has been no complaint.) My bash skill is not high, so my attempt to satisfy Comment #1 would be messy. Plese accept this pretext. ;-)

The submitted version is an improvement over my original code, by "steveL", contributed at this Gentoo forum post: https://forums.gentoo.org/viewtopic.php?p=8028864#8028864. steveL understands bash far better than me.

The patch applies against 1.4.5 <= elesect <= 1.4.8 and git-HEAD ATM.

[1] I don't know why it's not a test against stdout, but steveL seems to know tty well, too.

I've always been annoyed by the lack of pager support in "eselect news read". I'm glad with my contribution.

Thanks a lot Gentoo devs. Regards.
Comment 4 Ulrich Müller gentoo-dev 2017-02-16 23:22:08 UTC
Thank you for the patch.

Some additional work will be required to address the open questions from comment #1. Maybe just copying the logic from git is easiest as starting point (and we can always adjust things later).

Especially:

(In reply to Ulrich Müller from comment #1)
> - Presumably we need a --pager option that can override the environment.
>   (man has a -P option; git has -p/--paginate/--no-pager options.)

We need at least a --no-pager option, and an ESELECT_PAGER environment variable (which would override PAGER).

> - Not sure yet what to do with colour output. Not all pagers will accept it,
>   therefore disable colour when output is through a pager?

I still don't know what to do about this one. Maybe a lookup table which pagers can handle colour escape sequences and what options to pass (like -R for less).

> - News module only, or global feature?

Each module should decide for what actions pager support will be activated. So we can start with do_read() in the news module, and others can follow later.


(In reply to Teika kazura from comment #3)
> When stdin or stderr[1] is not attached to a terminal / console, then the
> pager won't be used.

> [1] I don't know why it's not a test against stdout, [...]

Definitely stdout should be tested there, not stderr. And stdin is irrelevant because this is about output redirection. So a simple [[ -t 1 ]] is the right thing to do. That's also what git does (look for isatty(1) in its pager.c).
Comment 5 Teika kazura 2017-02-17 12:11:00 UTC
> We need at least a --no-pager option,
Yes.

> an ESELECT_PAGER environment variable (which would override PAGER).
>
>> - Not sure yet what to do with colour output. Not all pagers will accept it,
>>   therefore disable colour when output is through a pager?
>
> I still don't know what to do about this one. Maybe a lookup table which pagers can handle colour escape sequences and what options to pass (like -R for less).

I'm gradually becoming doubtful about these points. When limited to "eselect news" you seldom run it, say twice a year or so. Excess coding complication, configurations, and work is better to avoid, and spare time for more important issues. Not sure about the whole eselect, but I don't need it often, either.

Although console etc are quite stable, they're difficult to learn, so for new developers of eselect, code with full coloring support will be obscure, making development burdensome.

To tell you the truth, I first tried to make it read a configuration file, which can be overrided by an environment variable. But I thought it was too complex, and chose single PAGER use. If users set PAGER, then they want it for most uses, I guess.

Thanks Ulrich Müller for your work in Gentoo. Regards.
Comment 6 Teika kazura 2020-04-09 02:26:30 UTC
Created attachment 631438 [details, diff]
Patch to support PAGER for "eselect news read".

This patch applies to app-admin/eselect-1.4.16. The previous patch doesn't.

Best regards.
Comment 7 Ulrich Müller gentoo-dev 2020-04-09 06:01:00 UTC
(In reply to Teika kazura from comment #6)
> Created attachment 631438 [details, diff] [details, diff]
> Patch to support PAGER for "eselect news read".

Looks like this patch doesn't address any of my items from comment #4?
Comment 8 Teika kazura 2020-04-10 01:15:06 UTC
Sorry, I'm not interested in any of your concern. All points seem paranoiac to me.

If one sets PAGER, that's what they want for 99%. (In your theory, environmental variales have to be banished from UNIX. Instead you have to set everything each and every time as a command line argument.) Anyway
----
alias eselect="PAGER=tac-or-whatever eselect"
----
could do the job.