[PATCH v4 5/6] am: Fix launching of pager

From

Jonathan Nieder <jrnieder@gmail.com>

Date

Feb 15, 2010, 05:04 UTC

Message-ID

<20100215050413.GA19586@progeny.tock>

In-Reply-To

<20100215044223.GA3336@coredump.intra.peff.net>

The pagination functionality in git am has some problems:

 - It does not check if stdout is a tty, so it always paginates.

 - If $GIT_PAGER uses any environment variables, they are being
   ignored, since it does not run $GIT_PAGER through eval.

 - If $GIT_PAGER is set to the empty string, instead of passing
   output through to stdout, it tries to run $dotest/patch.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Jeff King wrote:

> On Sun, Feb 14, 2010 at 09:25:33PM -0600, Jonathan Nieder wrote:

>> +		if tty <&1 >/dev/null 2>/dev/null
>
> We seem to use "test -t 1" for this elsewhere, and I don't see us using
> "tty" anywhere else. It is POSIX, and I tested that even Solaris 8 and
> AIX 6.1 have it. So I don't think it is a portability issue, but others
> may know more than I.

tty is portable, but "test -t 1" is cleaner.  Thanks for the pointer.

 git-am.sh       |    5 +----
 git-sh-setup.sh |   13 +++++++++++++
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index 3c08d53..b11af03 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -663,10 +663,7 @@ do
 		[eE]*) git_editor "$dotest/final-commit"
 		       action=again ;;
 		[vV]*) action=again
-		       : ${GIT_PAGER=$(git var GIT_PAGER)}
-		       : ${LESS=-FRSX}
-		       export LESS
-		       $GIT_PAGER "$dotest/patch" ;;
+		       git_pager "$dotest/patch" ;;
 		*)     action=again ;;
 		esac
 	    done
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index d56426d..44fb467 100755
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -107,6 +107,19 @@ git_editor() {
 	eval "$GIT_EDITOR" '"$@"'
 }
 
+git_pager() {
+	if test -t 1
+	then
+		GIT_PAGER=$(git var GIT_PAGER)
+	else
+		GIT_PAGER=cat
+	fi
+	: ${LESS=-FRSX}
+	export LESS
+
+	eval "$GIT_PAGER" '"$@"'
+}
+
 sane_grep () {
 	GREP_OPTIONS= LC_ALL=C grep "$@"
 }

-- 
1.7.0