git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Matthieu Moy <Matthieu.Moy@imag.fr>
To: git@vger.kernel.org, gitster@pobox.com
Cc: David Kastrup <dak@gnu.org>,
	d9ba@mailtor.net, jrnieder@gmail.com,
	Matthieu Moy <Matthieu.Moy@imag.fr>
Subject: [PATCH] PAGER_ENV: remove 'S' from $LESS by default
Date: Mon, 28 Apr 2014 10:34:22 +0200	[thread overview]
Message-ID: <1398674062-24288-1-git-send-email-Matthieu.Moy@imag.fr> (raw)
In-Reply-To: <20140425154722.GC11479@google.com>

By default, Git used to set $LESS to -FRSX if $LESS was not set by the
user. The FRX flags actually make sense for Git (F and X because Git
sometimes pipes short output to less, and R because Git pipes colored
output). The S flag (chop long lines), on the other hand, is not related
to Git and is a matter of user preference. Git should not decide for the
user to change LESS's default.

More specifically, the S flag harms users who review untrusted code
within a pager, since a patch looking like:

-old code;
+new good code; [... lots of tabs ...] malicious code;

would appear identical to:

-old code;
+new good code;

Users who prefer the old behavior can still set the $LESS environment
variable to -FRSX explicitly, or set core.pager to 'less -S'.

The documentation in config.txt is made a bit longer to keep both an
example setting the 'S' flag (needed to recover the old behavior) and an
example showing how to unset a flag set by Git.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
> We agree here.  So, does someone who actually wants this change want to
> propose a patch? :)

Here you are.

 Documentation/config.txt | 13 ++++++++-----
 Makefile                 |  6 +++---
 perl/Git/SVN/Log.pm      |  2 +-
 3 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index e30561d..b7f92ac 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -560,14 +560,17 @@ core.pager::
 	configuration, then `$PAGER`, and then the default chosen at
 	compile time (usually 'less').
 +
-When the `LESS` environment variable is unset, Git sets it to `FRSX`
+When the `LESS` environment variable is unset, Git sets it to `FRX`
 (if `LESS` environment variable is set, Git does not change it at
 all).  If you want to selectively override Git's default setting
-for `LESS`, you can set `core.pager` to e.g. `less -+S`.  This will
+for `LESS`, you can set `core.pager` to e.g. `less -S`.  This will
 be passed to the shell by Git, which will translate the final
-command to `LESS=FRSX less -+S`. The environment tells the command
-to set the `S` option to chop long lines but the command line
-resets it to the default to fold long lines.
+command to `LESS=FRX less -S`. The environment does not set the
+`S` option but the command line does, instructing less to truncate
+long lines. Similarly, setting `core.pager` to `less -+F` will
+deactivate the `F` option specified by the environment from the
+command-line, deactivating the "quit if one screen" behavior of
+`less`.
 +
 Likewise, when the `LV` environment variable is unset, Git sets it
 to `-c`.  You can override this setting by exporting `LV` with
diff --git a/Makefile b/Makefile
index a3b298e..cd3cdf6 100644
--- a/Makefile
+++ b/Makefile
@@ -344,9 +344,9 @@ all::
 # Define PAGER_ENV to a SP separated VAR=VAL pairs to define
 # default environment variables to be passed when a pager is spawned, e.g.
 #
-#    PAGER_ENV = LESS=-FRSX LV=-c
+#    PAGER_ENV = LESS=-FRX LV=-c
 #
-# to say "export LESS=-FRSX (and LV=-c) if the environment variable
+# to say "export LESS=-FRX (and LV=-c) if the environment variable
 # LESS (and LV) is not set, respectively".
 
 GIT-VERSION-FILE: FORCE
@@ -1518,7 +1518,7 @@ NO_PYTHON = NoThanks
 endif
 
 ifndef PAGER_ENV
-PAGER_ENV = LESS=-FRSX LV=-c
+PAGER_ENV = LESS=-FRX LV=-c
 endif
 
 QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
diff --git a/perl/Git/SVN/Log.pm b/perl/Git/SVN/Log.pm
index 34f2869..6641053 100644
--- a/perl/Git/SVN/Log.pm
+++ b/perl/Git/SVN/Log.pm
@@ -116,7 +116,7 @@ sub run_pager {
 		return;
 	}
 	open STDIN, '<&', $rfd or fatal "Can't redirect stdin: $!";
-	$ENV{LESS} ||= 'FRSX';
+	$ENV{LESS} ||= 'FRX';
 	$ENV{LV} ||= '-c';
 	exec $pager or fatal "Can't run pager: $! ($pager)";
 }
-- 
1.9.2.698.ge58c0c2.dirty

  reply	other threads:[~2014-04-28  8:36 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-23 23:46 Harmful LESS flags d9ba
2014-04-24  0:11 ` Jonathan Nieder
2014-04-28 21:38   ` Mark Nudelman
2014-04-24  5:06 ` David Kastrup
2014-04-24 19:02   ` Junio C Hamano
2014-04-24 19:21     ` David Kastrup
2014-04-24 19:29       ` Junio C Hamano
2014-04-24 19:50         ` David Kastrup
2014-04-24 21:35         ` Jeff King
2014-04-24 21:47           ` Junio C Hamano
2014-04-24 22:02             ` Jeff King
2014-04-24 21:48           ` David Kastrup
2014-04-24 22:13             ` Jeff King
2014-04-24 22:44               ` David Kastrup
2014-04-24 23:08                 ` Jonathan Nieder
2014-04-25  6:56     ` Matthieu Moy
2014-04-25 15:11       ` Jonathan Nieder
2014-04-25 15:32         ` David Kastrup
2014-04-25 15:47           ` Jonathan Nieder
2014-04-28  8:34             ` Matthieu Moy [this message]
2014-04-28  8:43               ` [PATCH] PAGER_ENV: remove 'S' from $LESS by default David Kastrup
2014-04-28  8:59                 ` Matthieu Moy
2014-04-28  9:14                   ` David Kastrup
2014-04-28 12:22                     ` Matthieu Moy
2014-04-28 16:24                       ` Jeff King
2014-04-28 18:48                         ` Junio C Hamano
2014-04-29 12:29                           ` Matthieu Moy
2014-04-29 17:01                             ` Junio C Hamano
2014-04-30  7:35                               ` [PATCH v2] pager: " Matthieu Moy
2014-04-30 15:38                                 ` Junio C Hamano
2014-04-30 15:49                                   ` Matthieu Moy
2014-04-30 17:34                                     ` Junio C Hamano
2014-05-05 18:44                                 ` Jonathan Nieder
2014-05-05 20:10                                   ` Matthieu Moy
2014-05-06 17:34                                     ` Junio C Hamano
2014-05-06 18:00                                       ` David Kastrup
2014-05-06 18:49                                         ` Matthieu Moy
2014-05-06 21:55                                           ` Jeff King
2014-05-07 17:07                                             ` Junio C Hamano
2014-05-07 17:54                                               ` Matthieu Moy
2014-05-07 20:42                                                 ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1398674062-24288-1-git-send-email-Matthieu.Moy@imag.fr \
    --to=matthieu.moy@imag.fr \
    --cc=d9ba@mailtor.net \
    --cc=dak@gnu.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).