# [PATCH] request-pull: do not paginate output of git commands 7 messages from 2009-06-30 to 2009-07-01. Participants: Michal Marek, Junio C Hamano. Thread: https://gitlist.dev/t/19980 ## Michal Marek, 2009-06-30 11:33 Subject: [PATCH] request-pull: do not paginate output of git commands Message-ID: <1246361606-20457-1-git-send-email-mmarek@suse.cz> URL: https://gitlist.dev/e/1246361606-20457-1-git-send-email-mmarek%40suse.cz ``` git request-pull called inside a terminal prints part of the output to the terminal and other parts are piped through the pager. Fix this. Signed-off-by: Michal Marek --- git-request-pull.sh | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/git-request-pull.sh b/git-request-pull.sh index a2cf5b8..b3aaded 100755 --- a/git-request-pull.sh +++ b/git-request-pull.sh @@ -34,7 +34,7 @@ branch=$(git ls-remote "$url" \ }") if [ -z "$branch" ]; then echo "warn: No branch of $url is at:" >&2 - git log --max-count=1 --pretty='format:warn: %h: %s' $headrev >&2 + git --no-pager log --max-count=1 --pretty='tformat:warn: %h: %s' $headrev >&2 echo "warn: Are you sure you pushed $head there?" >&2 echo >&2 echo >&2 @@ -45,13 +45,13 @@ fi PAGER= export PAGER echo "The following changes since commit $baserev:" -git shortlog --max-count=1 $baserev | sed -e 's/^\(.\)/ \1/' +git --no-pager shortlog --max-count=1 $baserev | sed -e 's/^\(.\)/ \1/' echo "are available in the git repository at:" echo echo " $url $branch" echo -git shortlog ^$baserev $headrev -git diff -M --stat --summary $merge_base $headrev +git --no-pager shortlog ^$baserev $headrev +git --no-pager diff -M --stat --summary $merge_base $headrev exit $status -- 1.6.3 ``` ## Junio C Hamano, 2009-06-30 16:20 Subject: Re: [PATCH] request-pull: do not paginate output of git commands Message-ID: <7vws6t8zhv.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vws6t8zhv.fsf%40alter.siamese.dyndns.org In-Reply-To: <1246361606-20457-1-git-send-email-mmarek@suse.cz> ``` Michal Marek writes: > git request-pull called inside a terminal prints part of the output to > the terminal and other parts are piped through the pager. Fix this. Hmph, I have always thought this was meant to be a feature. That is, you run it to yourself, instead of piping it to a file or "| mail torvalds", in order to sanity check before you actually do the latter. When the request is larger than a screenful, you would want a pager while reviewing. Admittedly, I do not regularly use request-pull, so I wouldn't know if use of the pager is inconvenient to people who have to use this feature regularly. Your patch could be addressing a real usability issue. But I have this nagging suspicion that you may be lacking F (or FX) from your LESS environment variable? ``` ## Junio C Hamano, 2009-06-30 16:32 Subject: Re: [PATCH] request-pull: do not paginate output of git commands Message-ID: <7vskhh8yxf.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vskhh8yxf.fsf%40alter.siamese.dyndns.org In-Reply-To: <7vws6t8zhv.fsf@alter.siamese.dyndns.org> ``` Junio C Hamano writes: > Michal Marek writes: > >> git request-pull called inside a terminal prints part of the output to >> the terminal and other parts are piped through the pager. Fix this. > > Hmph, I have always thought this was meant to be a feature. > > That is, you run it to yourself, instead of piping it to a file or "| mail > torvalds", in order to sanity check before you actually do the latter. > When the request is larger than a screenful, you would want a pager while > reviewing. Sorry, I take it back. If it _were_ a single call to a paging command what I said may make sense, but with many separate calls to shortlog, we wouldn't know which output is too large (and uses pager). I agree your patch makes things better. I should have read your commit log message after finishing my coffee ;-) Thanks. ``` ## Michal Marek, 2009-06-30 17:26 Subject: Re: [PATCH] request-pull: do not paginate output of git commands Message-ID: <4A4A4AB3.8030107@suse.cz> URL: https://gitlist.dev/e/4A4A4AB3.8030107%40suse.cz In-Reply-To: <7vskhh8yxf.fsf@alter.siamese.dyndns.org> ``` Junio C Hamano napsal(a): > Junio C Hamano writes: > >> Michal Marek writes: >> >>> git request-pull called inside a terminal prints part of the output to >>> the terminal and other parts are piped through the pager. Fix this. >> Hmph, I have always thought this was meant to be a feature. >> >> That is, you run it to yourself, instead of piping it to a file or "| mail >> torvalds", in order to sanity check before you actually do the latter. >> When the request is larger than a screenful, you would want a pager while >> reviewing. > > Sorry, I take it back. > > If it _were_ a single call to a paging command what I said may make sense, > but with many separate calls to shortlog, we wouldn't know which output is > too large (and uses pager). I agree your patch makes things better. Yes, this is what I dislike, it prints parts to the terminal and parts are paged (separately). Paging the whole output would be a nice improvement, but probably not worth the effort. > I should have read your commit log message after finishing my coffee ;-) :-) Michal ``` ## Junio C Hamano, 2009-06-30 18:28 Subject: Re: [PATCH] request-pull: do not paginate output of git commands Message-ID: <7vtz1x7f05.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vtz1x7f05.fsf%40alter.siamese.dyndns.org In-Reply-To: <1246361606-20457-1-git-send-email-mmarek@suse.cz> ``` Michal Marek writes: > git request-pull called inside a terminal prints part of the output to > the terminal and other parts are piped through the pager. Fix this. > > Signed-off-by: Michal Marek > --- Having said all that, I noticed this gem in the context in your patch. > git-request-pull.sh | 8 ++++---- > 1 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/git-request-pull.sh b/git-request-pull.sh > index a2cf5b8..b3aaded 100755 > --- a/git-request-pull.sh > +++ b/git-request-pull.sh > @@ -45,13 +45,13 @@ fi > PAGER= > export PAGER > echo "The following changes since commit $baserev:" Looking at ff06c74 (Improve request-pull to handle non-rebased branches, 2007-05-01), it introduced these specifically to address the issue you are bringing up, but it is ineffective. I'd suggest to replace your patch with the attached instead. --- git-request-pull.sh | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/git-request-pull.sh b/git-request-pull.sh index a2cf5b8..f86d8fd 100755 --- a/git-request-pull.sh +++ b/git-request-pull.sh @@ -42,8 +42,8 @@ if [ -z "$branch" ]; then status=1 fi -PAGER= -export PAGER +GIT_PAGER=cat +export GIT_PAGER echo "The following changes since commit $baserev:" git shortlog --max-count=1 $baserev | sed -e 's/^\(.\)/ \1/' ``` ## Michal Marek, 2009-07-01 09:40 Subject: Re: [PATCH] request-pull: do not paginate output of git commands Message-ID: <20090701094029.GA22508@sepie.suse.cz> URL: https://gitlist.dev/e/20090701094029.GA22508%40sepie.suse.cz In-Reply-To: <7vtz1x7f05.fsf@alter.siamese.dyndns.org> ``` On Tue, Jun 30, 2009 at 11:28:26AM -0700, Junio C Hamano wrote: > Looking at ff06c74 (Improve request-pull to handle non-rebased branches, > 2007-05-01), it introduced these specifically to address the issue you are > bringing up, but it is ineffective. > > I'd suggest to replace your patch with the attached instead. > > --- > > git-request-pull.sh | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/git-request-pull.sh b/git-request-pull.sh > index a2cf5b8..f86d8fd 100755 > --- a/git-request-pull.sh > +++ b/git-request-pull.sh > @@ -42,8 +42,8 @@ if [ -z "$branch" ]; then > status=1 > fi > > -PAGER= > -export PAGER > +GIT_PAGER=cat > +export GIT_PAGER Good idea, but for completeness, it should be set before this command: git log --max-count=1 --pretty='format:warn: %h: %s' $headrev >&2 (which should use tformat to get the newline right). So what about this one? --- diff --git a/git-request-pull.sh b/git-request-pull.sh index ab2dd10..5917773 100755 --- a/git-request-pull.sh +++ b/git-request-pull.sh @@ -12,6 +12,9 @@ OPTIONS_SPEC= . git-sh-setup . git-parse-remote +GIT_PAGER= +export GIT_PAGER + base=$1 url=$2 head=${3-HEAD} @@ -34,7 +37,7 @@ branch=$(git ls-remote "$url" \ }") if [ -z "$branch" ]; then echo "warn: No branch of $url is at:" >&2 - git log --max-count=1 --pretty='format:warn: %h: %s' $headrev >&2 + git log --max-count=1 --pretty='tformat:warn: %h: %s' $headrev >&2 echo "warn: Are you sure you pushed $head there?" >&2 echo >&2 echo >&2 @@ -42,8 +45,6 @@ if [ -z "$branch" ]; then status=1 fi -GIT_PAGER= -export GIT_PAGER echo "The following changes since commit $baserev:" git shortlog --max-count=1 $baserev | sed -e 's/^\(.\)/ \1/' ``` ## Junio C Hamano, 2009-07-01 20:16 Subject: Re: [PATCH] request-pull: do not paginate output of git commands Message-ID: <7vy6r8f9aq.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vy6r8f9aq.fsf%40alter.siamese.dyndns.org In-Reply-To: <20090701094029.GA22508@sepie.suse.cz> ``` Michal Marek writes: > Good idea, but for completeness, it should be set before this command: > git log --max-count=1 --pretty='format:warn: %h: %s' $headrev >&2 > (which should use tformat to get the newline right). So what about this > one? Good eyes. The blamed commit for that new use of "log" is ff06c74 (Improve request-pull to handle non-rebased branches, 2007-05-01) itself, which did introduce the "define an empty PAGER to disable paging" trick, so this makes it doubly buggy (i.e. for forgetting GIT_PAGER trumps PAGER, and for introducing a use of log that is not covered by its own trick). Thanks. ```