{"thread":{"id":"19980","subject":"[PATCH] request-pull: do not paginate output of git commands","startedAt":"2009-06-30T11:33:26Z","lastAt":"2009-07-01T20:16:45Z","messageCount":7,"participants":["Michal Marek","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"117228","messageId":"1246361606-20457-1-git-send-email-mmarek@suse.cz","threadId":"19980","inReplyTo":null,"subject":"[PATCH] request-pull: do not paginate output of git commands","fromName":"Michal Marek","fromEmail":"mmarek@suse.cz","sentAt":"2009-06-30T11:33:26Z","receivedAt":"2009-06-30T11:33:26Z","isPatch":true,"sender":{"key":"mmarek@suse.cz","avatar":"https://gravatar.com/avatar/b026025af60a27d7ea0635833f351de6b0a6089a9573415d86c9352eda4f6e9a?d=mp&s=160"},"body":"git request-pull called inside a terminal prints part of the output to\nthe terminal and other parts are piped through the pager. Fix this.\n\nSigned-off-by: Michal Marek \n---\n git-request-pull.sh | 8 ++++----\n 1 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/git-request-pull.sh b/git-request-pull.sh\nindex a2cf5b8..b3aaded 100755\n--- a/git-request-pull.sh\n+++ b/git-request-pull.sh\n@@ -34,7 +34,7 @@ branch=$(git ls-remote \"$url\" \\\n \t}\")\n if [ -z \"$branch\" ]; then\n \techo \"warn: No branch of $url is at:\" >&2\n-\tgit log --max-count=1 --pretty='format:warn: %h: %s' $headrev >&2\n+\tgit --no-pager log --max-count=1 --pretty='tformat:warn: %h: %s' $headrev >&2\n \techo \"warn: Are you sure you pushed $head there?\" >&2\n \techo >&2\n \techo >&2\n@@ -45,13 +45,13 @@ fi\n PAGER=\n export PAGER\n echo \"The following changes since commit $baserev:\"\n-git shortlog --max-count=1 $baserev | sed -e 's/^\\(.\\)/ \\1/'\n+git --no-pager shortlog --max-count=1 $baserev | sed -e 's/^\\(.\\)/ \\1/'\n \n echo \"are available in the git repository at:\"\n echo\n echo \" $url $branch\"\n echo\n \n-git shortlog ^$baserev $headrev\n-git diff -M --stat --summary $merge_base $headrev\n+git --no-pager shortlog ^$baserev $headrev\n+git --no-pager diff -M --stat --summary $merge_base $headrev\n exit $status\n-- \n1.6.3\n"},{"id":"117241","messageId":"7vws6t8zhv.fsf@alter.siamese.dyndns.org","threadId":"19980","inReplyTo":"1246361606-20457-1-git-send-email-mmarek@suse.cz","subject":"Re: [PATCH] request-pull: do not paginate output of git commands","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-06-30T16:20:28Z","receivedAt":"2009-06-30T16:20:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michal Marek writes:\n\n> git request-pull called inside a terminal prints part of the output to\n> the terminal and other parts are piped through the pager. Fix this.\n\nHmph, I have always thought this was meant to be a feature.\n\nThat is, you run it to yourself, instead of piping it to a file or \"| mail\ntorvalds\", in order to sanity check before you actually do the latter.\nWhen the request is larger than a screenful, you would want a pager while\nreviewing.\n\nAdmittedly, I do not regularly use request-pull, so I wouldn't know if use\nof the pager is inconvenient to people who have to use this feature\nregularly. Your patch could be addressing a real usability issue.\n\nBut I have this nagging suspicion that you may be lacking F (or FX) from\nyour LESS environment variable?\n"},{"id":"117242","messageId":"7vskhh8yxf.fsf@alter.siamese.dyndns.org","threadId":"19980","inReplyTo":"7vws6t8zhv.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] request-pull: do not paginate output of git commands","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-06-30T16:32:44Z","receivedAt":"2009-06-30T16:32:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Michal Marek writes:\n>\n>> git request-pull called inside a terminal prints part of the output to\n>> the terminal and other parts are piped through the pager. Fix this.\n>\n> Hmph, I have always thought this was meant to be a feature.\n>\n> That is, you run it to yourself, instead of piping it to a file or \"| mail\n> torvalds\", in order to sanity check before you actually do the latter.\n> When the request is larger than a screenful, you would want a pager while\n> reviewing.\n\nSorry, I take it back.\n\nIf it _were_ a single call to a paging command what I said may make sense,\nbut with many separate calls to shortlog, we wouldn't know which output is\ntoo large (and uses pager). I agree your patch makes things better.\n\nI should have read your commit log message after finishing my coffee ;-)\n\nThanks.\n"},{"id":"117244","messageId":"4A4A4AB3.8030107@suse.cz","threadId":"19980","inReplyTo":"7vskhh8yxf.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] request-pull: do not paginate output of git commands","fromName":"Michal Marek","fromEmail":"mmarek@suse.cz","sentAt":"2009-06-30T17:26:11Z","receivedAt":"2009-06-30T17:26:11Z","isPatch":true,"sender":{"key":"mmarek@suse.cz","avatar":"https://gravatar.com/avatar/b026025af60a27d7ea0635833f351de6b0a6089a9573415d86c9352eda4f6e9a?d=mp&s=160"},"body":"Junio C Hamano napsal(a):\n> Junio C Hamano writes:\n> \n>> Michal Marek writes:\n>>\n>>> git request-pull called inside a terminal prints part of the output to\n>>> the terminal and other parts are piped through the pager. Fix this.\n>> Hmph, I have always thought this was meant to be a feature.\n>>\n>> That is, you run it to yourself, instead of piping it to a file or \"| mail\n>> torvalds\", in order to sanity check before you actually do the latter.\n>> When the request is larger than a screenful, you would want a pager while\n>> reviewing.\n> \n> Sorry, I take it back.\n> \n> If it _were_ a single call to a paging command what I said may make sense,\n> but with many separate calls to shortlog, we wouldn't know which output is\n> too large (and uses pager). I agree your patch makes things better.\n\nYes, this is what I dislike, it prints parts to the terminal and parts\nare paged (separately). Paging the whole output would be a nice\nimprovement, but probably not worth the effort.\n\n\n> I should have read your commit log message after finishing my coffee ;-)\n\n:-)\n\nMichal\n"},{"id":"117247","messageId":"7vtz1x7f05.fsf@alter.siamese.dyndns.org","threadId":"19980","inReplyTo":"1246361606-20457-1-git-send-email-mmarek@suse.cz","subject":"Re: [PATCH] request-pull: do not paginate output of git commands","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-06-30T18:28:26Z","receivedAt":"2009-06-30T18:28:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michal Marek writes:\n\n> git request-pull called inside a terminal prints part of the output to\n> the terminal and other parts are piped through the pager. Fix this.\n>\n> Signed-off-by: Michal Marek \n> ---\n\nHaving said all that, I noticed this gem in the context in your patch.\n\n> git-request-pull.sh | 8 ++++----\n> 1 files changed, 4 insertions(+), 4 deletions(-)\n>\n> diff --git a/git-request-pull.sh b/git-request-pull.sh\n> index a2cf5b8..b3aaded 100755\n> --- a/git-request-pull.sh\n> +++ b/git-request-pull.sh\n> @@ -45,13 +45,13 @@ fi\n> PAGER=\n> export PAGER\n> echo \"The following changes since commit $baserev:\"\n\nLooking at ff06c74 (Improve request-pull to handle non-rebased branches,\n2007-05-01), it introduced these specifically to address the issue you are\nbringing up, but it is ineffective.\n\nI'd suggest to replace your patch with the attached instead.\n\n---\n\n git-request-pull.sh | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/git-request-pull.sh b/git-request-pull.sh\nindex a2cf5b8..f86d8fd 100755\n--- a/git-request-pull.sh\n+++ b/git-request-pull.sh\n@@ -42,8 +42,8 @@ if [ -z \"$branch\" ]; then\n \tstatus=1\n fi\n \n-PAGER=\n-export PAGER\n+GIT_PAGER=cat\n+export GIT_PAGER\n echo \"The following changes since commit $baserev:\"\n git shortlog --max-count=1 $baserev | sed -e 's/^\\(.\\)/ \\1/'\n \n"},{"id":"117297","messageId":"20090701094029.GA22508@sepie.suse.cz","threadId":"19980","inReplyTo":"7vtz1x7f05.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] request-pull: do not paginate output of git commands","fromName":"Michal Marek","fromEmail":"mmarek@suse.cz","sentAt":"2009-07-01T09:40:30Z","receivedAt":"2009-07-01T09:40:30Z","isPatch":true,"sender":{"key":"mmarek@suse.cz","avatar":"https://gravatar.com/avatar/b026025af60a27d7ea0635833f351de6b0a6089a9573415d86c9352eda4f6e9a?d=mp&s=160"},"body":"On Tue, Jun 30, 2009 at 11:28:26AM -0700, Junio C Hamano wrote:\n> Looking at ff06c74 (Improve request-pull to handle non-rebased branches,\n> 2007-05-01), it introduced these specifically to address the issue you are\n> bringing up, but it is ineffective.\n> \n> I'd suggest to replace your patch with the attached instead.\n> \n> ---\n> \n> git-request-pull.sh | 4 ++--\n> 1 files changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/git-request-pull.sh b/git-request-pull.sh\n> index a2cf5b8..f86d8fd 100755\n> --- a/git-request-pull.sh\n> +++ b/git-request-pull.sh\n> @@ -42,8 +42,8 @@ if [ -z \"$branch\" ]; then\n> \tstatus=1\n> fi\n> \n> -PAGER=\n> -export PAGER\n> +GIT_PAGER=cat\n> +export GIT_PAGER\n\nGood idea, but for completeness, it should be set before this command:\n git log --max-count=1 --pretty='format:warn: %h: %s' $headrev >&2\n(which should use tformat to get the newline right). So what about this\none?\n\n---\n\ndiff --git a/git-request-pull.sh b/git-request-pull.sh\nindex ab2dd10..5917773 100755\n--- a/git-request-pull.sh\n+++ b/git-request-pull.sh\n@@ -12,6 +12,9 @@ OPTIONS_SPEC=\n . git-sh-setup\n . git-parse-remote\n \n+GIT_PAGER=\n+export GIT_PAGER\n+\n base=$1\n url=$2\n head=${3-HEAD}\n@@ -34,7 +37,7 @@ branch=$(git ls-remote \"$url\" \\\n \t}\")\n if [ -z \"$branch\" ]; then\n \techo \"warn: No branch of $url is at:\" >&2\n-\tgit log --max-count=1 --pretty='format:warn: %h: %s' $headrev >&2\n+\tgit log --max-count=1 --pretty='tformat:warn: %h: %s' $headrev >&2\n \techo \"warn: Are you sure you pushed $head there?\" >&2\n \techo >&2\n \techo >&2\n@@ -42,8 +45,6 @@ if [ -z \"$branch\" ]; then\n \tstatus=1\n fi\n \n-GIT_PAGER=\n-export GIT_PAGER\n echo \"The following changes since commit $baserev:\"\n git shortlog --max-count=1 $baserev | sed -e 's/^\\(.\\)/ \\1/'\n \n"},{"id":"117317","messageId":"7vy6r8f9aq.fsf@alter.siamese.dyndns.org","threadId":"19980","inReplyTo":"20090701094029.GA22508@sepie.suse.cz","subject":"Re: [PATCH] request-pull: do not paginate output of git commands","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-07-01T20:16:45Z","receivedAt":"2009-07-01T20:16:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michal Marek writes:\n\n> Good idea, but for completeness, it should be set before this command:\n> git log --max-count=1 --pretty='format:warn: %h: %s' $headrev >&2\n> (which should use tformat to get the newline right). So what about this\n> one?\n\nGood eyes.\n\nThe blamed commit for that new use of \"log\" is ff06c74 (Improve\nrequest-pull to handle non-rebased branches, 2007-05-01) itself, which did\nintroduce the \"define an empty PAGER to disable paging\" trick, so this\nmakes it doubly buggy (i.e. for forgetting GIT_PAGER trumps PAGER, and for\nintroducing a use of log that is not covered by its own trick).\n\nThanks.\n"}]}