threads / patch / 19980

patchrequest-pull: do not paginate output of git commands

Subject: [PATCH] request-pull: do not paginate output of git commands

## tl;dr

7 messages between Jun 30, 2009 and Jul 1, 2009. Diffs are folded; open one to read it.

replies: 6people: 2as markdown or json

Michal Marek· Jun 30, 2009, 11:33 UTC · lore

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 <mmarek@suse.cz>
---
 git-request-pull.sh |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)
Show changes to git-request-pull.sh +4 −4
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· Jun 30, 2009, 16:20 UTC · re: Michal Marek · lore

Re: [PATCH] request-pull: do not paginate output of git commands

Michal Marek <mmarek@suse.cz> 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· Jun 30, 2009, 16:32 UTC · re: Junio C Hamano · lore

Re: [PATCH] request-pull: do not paginate output of git commands

Junio C Hamano <gitster@pobox.com> writes:
Show 11 quoted lines
> Michal Marek <mmarek@suse.cz> 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· Jun 30, 2009, 17:26 UTC · re: Junio C Hamano · lore

Re: [PATCH] request-pull: do not paginate output of git commands

Junio C Hamano napsal(a):
Show 18 quoted lines
> Junio C Hamano <gitster@pobox.com> writes:
> 
>> Michal Marek <mmarek@suse.cz> 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· Jun 30, 2009, 18:28 UTC · re: Michal Marek · lore

Re: [PATCH] request-pull: do not paginate output of git commands

Michal Marek <mmarek@suse.cz> writes:
Show 5 quoted lines
> 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 <mmarek@suse.cz>
> ---
Having said all that, I noticed this gem in the context in your patch.
Show 11 quoted lines
>  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(-)
Show changes to git-request-pull.sh +2 −2
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· Jul 1, 2009, 09:40 UTC · re: Junio C Hamano · lore

Re: [PATCH] request-pull: do not paginate output of git commands

On Tue, Jun 30, 2009 at 11:28:26AM -0700, Junio C Hamano wrote:
Show 23 quoted lines
> 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?
---
Show changes to git-request-pull.sh +4 −3
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· Jul 1, 2009, 20:16 UTC · re: Michal Marek · lore

Re: [PATCH] request-pull: do not paginate output of git commands

Michal Marek <mmarek@suse.cz> 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.

← back to recent threads