Patchwork [2,of,2] pager: skip running the pager if it's set to 'cat'

login
register
mail settings
Submitter Augie Fackler
Date March 16, 2017, 1:44 a.m.
Message ID <bc0c38ec3f7be807607d.1489628666@augie-macbookair2.roam.corp.google.com>
Download mbox | patch
Permalink /patch/19371/
State Accepted
Headers show

Comments

Augie Fackler - March 16, 2017, 1:44 a.m.
# HG changeset patch
# User Augie Fackler <augie@google.com>
# Date 1489624466 14400
#      Wed Mar 15 20:34:26 2017 -0400
# Node ID bc0c38ec3f7be807607d4fdf871f344e7079d992
# Parent  07d488f16da6e12b225d2827f1020f32c8050a7a
pager: skip running the pager if it's set to 'cat'

Avoid useless uses of cat.
Ryan McElroy - March 16, 2017, 2:07 a.m.
On 3/15/17 6:44 PM, Augie Fackler wrote:
> # HG changeset patch
> # User Augie Fackler <augie@google.com>
> # Date 1489624466 14400
> #      Wed Mar 15 20:34:26 2017 -0400
> # Node ID bc0c38ec3f7be807607d4fdf871f344e7079d992
> # Parent  07d488f16da6e12b225d2827f1020f32c8050a7a
> pager: skip running the pager if it's set to 'cat'
>
> Avoid useless uses of cat.

I hate myself a little for being this pedantic... but if 'cat' means 
something else in a users environment (eg, alias cat="cat -v"), then 
this would break that behavior.

>
> diff --git a/mercurial/ui.py b/mercurial/ui.py
> --- a/mercurial/ui.py
> +++ b/mercurial/ui.py
> @@ -935,6 +935,9 @@ class ui(object):
>           This is separate in part so that extensions (like chg) can
>           override how a pager is invoked.
>           """
> +        if command == 'cat':
> +            # Save ourselves some work.
> +            return
>           # If the command doesn't contain any of these characters, we
>           # assume it's a binary and exec it directly. This means for
>           # simple pager command configurations, we can degrade
>
Augie Fackler - March 16, 2017, 4:04 a.m.
> On Mar 15, 2017, at 20:17, Kevin Bullock <kbullock+mercurial@ringworld.org> wrote:
> 
>> On Mar 15, 2017, at 21:07, Ryan McElroy <rm@fb.com> wrote:
>> 
>> On 3/15/17 6:44 PM, Augie Fackler wrote:
>>> # HG changeset patch
>>> # User Augie Fackler <augie@google.com>
>>> # Date 1489624466 14400
>>> #      Wed Mar 15 20:34:26 2017 -0400
>>> # Node ID bc0c38ec3f7be807607d4fdf871f344e7079d992
>>> # Parent  07d488f16da6e12b225d2827f1020f32c8050a7a
>>> pager: skip running the pager if it's set to 'cat'
>>> 
>>> Avoid useless uses of cat.
>> 
>> I hate myself a little for being this pedantic... but if 'cat' means something else in a users environment (eg, alias cat="cat -v"), then this would break that behavior.
> 
> \emperorpalpatine{Heh heh heh. Good! ... Your hate has made you powerful.}

Patch 1 already breaks that, so if we decide the incremental improvement of patch 1 is worth the change, then we can probably do this.

Note that git already hardcodes this ;)
Jun Wu - March 16, 2017, 4:12 a.m.
I think these changes are good. I'd like to have more "modernize" stuffs.

Another thing that git does (reasonably, I think) is to set default
environment variable. Say if we decide to go with shell=False, set LESS=FRX
so people using pager.pager=less will have a better experience.

Excerpts from Augie Fackler's message of 2017-03-15 21:44:26 -0400:
> # HG changeset patch
> # User Augie Fackler <augie@google.com>
> # Date 1489624466 14400
> #      Wed Mar 15 20:34:26 2017 -0400
> # Node ID bc0c38ec3f7be807607d4fdf871f344e7079d992
> # Parent  07d488f16da6e12b225d2827f1020f32c8050a7a
> pager: skip running the pager if it's set to 'cat'
> 
> Avoid useless uses of cat.
> 
> diff --git a/mercurial/ui.py b/mercurial/ui.py
> --- a/mercurial/ui.py
> +++ b/mercurial/ui.py
> @@ -935,6 +935,9 @@ class ui(object):
>          This is separate in part so that extensions (like chg) can
>          override how a pager is invoked.
>          """
> +        if command == 'cat':
> +            # Save ourselves some work.
> +            return
>          # If the command doesn't contain any of these characters, we
>          # assume it's a binary and exec it directly. This means for
>          # simple pager command configurations, we can degrade
Augie Fackler - March 16, 2017, 4:44 p.m.
> On Mar 15, 2017, at 21:12, Jun Wu <quark@fb.com> wrote:
> 
> Another thing that git does (reasonably, I think) is to set default
> environment variable. Say if we decide to go with shell=False, set LESS=FRX
> so people using pager.pager=less will have a better experience.

We should add that environment variable iff it's not already set in the user's environment, of course.
Yuya Nishihara - March 17, 2017, 2:22 p.m.
On Wed, 15 Mar 2017 21:44:26 -0400, Augie Fackler wrote:
> # HG changeset patch
> # User Augie Fackler <augie@google.com>
> # Date 1489624466 14400
> #      Wed Mar 15 20:34:26 2017 -0400
> # Node ID bc0c38ec3f7be807607d4fdf871f344e7079d992
> # Parent  07d488f16da6e12b225d2827f1020f32c8050a7a
> pager: skip running the pager if it's set to 'cat'

The series looks good, queued, thanks.

I have no strong feeling about 'cat' thing. I hope nobody would install
nyancat as cat.
Gregory Szorc - March 18, 2017, 5:25 p.m.
On Thu, Mar 16, 2017 at 9:44 AM, Augie Fackler <raf@durin42.com> wrote:

>
> On Mar 15, 2017, at 21:12, Jun Wu <quark@fb.com> wrote:
>
> Another thing that git does (reasonably, I think) is to set default
> environment variable. Say if we decide to go with shell=False, set LESS=FRX
> so people using pager.pager=less will have a better experience.
>
>
> We should add that environment variable iff it's not already set in the
> user's environment, of course.
>

+1

I'll also 2nd LESS=FRX as a reasonable default. That is:

-F quit if output smaller than one screen
-R emit ansi color sequences (but not other raw control characters)
-X do not send termap init and deinit strings to terminal


>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
>

Patch

diff --git a/mercurial/ui.py b/mercurial/ui.py
--- a/mercurial/ui.py
+++ b/mercurial/ui.py
@@ -935,6 +935,9 @@  class ui(object):
         This is separate in part so that extensions (like chg) can
         override how a pager is invoked.
         """
+        if command == 'cat':
+            # Save ourselves some work.
+            return
         # If the command doesn't contain any of these characters, we
         # assume it's a binary and exec it directly. This means for
         # simple pager command configurations, we can degrade