Thread (1 message) 1 message, 1 author, 2016-06-15

Re: [PATCH v2] pager: remove 'S' from $LESS by default

From: Junio C Hamano <hidden>
Date: 2016-06-15 23:00:56

Matthieu Moy [off-list ref] writes:
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.
Git always pipes its output to less, not just "sometimes pipes short
ones" ;-)  "because the output may be short" would be more accurate
and would convey the same thing.

But that is just nitpicking.  The patch looks totally agreeable.

I am inclined to suggest queuing it for the first batch after 2.0
instead of directly applying to 'master', as we have past the point
we can expect to see reports of unexpected fallouts and fix the
issues in time for the final.

Thanks.
quoted hunk
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 <redacted>
---
This is just a rewrite of PATCH v1 on top of master instead of pu.

 Documentation/config.txt | 13 ++++++++-----
 git-sh-setup.sh          |  2 +-
 pager.c                  |  2 +-
 perl/Git/SVN/Log.pm      |  2 +-
 4 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 73c8973..5484d9d 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -553,14 +553,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/git-sh-setup.sh b/git-sh-setup.sh
index 5f28b32..9447980 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -160,7 +160,7 @@ git_pager() {
 	else
 		GIT_PAGER=cat
 	fi
-	: ${LESS=-FRSX}
+	: ${LESS=-FRX}
 	: ${LV=-c}
 	export LESS LV
 
diff --git a/pager.c b/pager.c
index 0cc75a8..f75e8ae 100644
--- a/pager.c
+++ b/pager.c
@@ -85,7 +85,7 @@ void setup_pager(void)
 		int i = 0;
 
 		if (!getenv("LESS"))
-			env[i++] = "LESS=FRSX";
+			env[i++] = "LESS=FRX";
 		if (!getenv("LV"))
 			env[i++] = "LV=-c";
 		env[i] = NULL;
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)";
 }
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help