| 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
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 >
> 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 ;)
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
> 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.
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.
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