{"thread":{"id":"36489","subject":"[PATCH] git-request-pull: add --stat option","startedAt":"2014-04-24T09:29:23Z","lastAt":"2014-04-24T17:46:25Z","messageCount":2,"participants":["Jiri Slaby","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"239549","messageId":"1398331763-601-1-git-send-email-jslaby@suse.cz","threadId":"36489","inReplyTo":null,"subject":"[PATCH] git-request-pull: add --stat option","fromName":"Jiri Slaby","fromEmail":"jslaby@suse.cz","sentAt":"2014-04-24T09:29:23Z","receivedAt":"2014-04-24T09:29:23Z","isPatch":true,"sender":{"key":"jslaby@suse.cz","avatar":null},"body":"Which is passed on to git diff. I very need this option instead of\nchanging the terminal size.\n\nSigned-off-by: Jiri Slaby <jslaby@suse.cz>\n---\n git-request-pull.sh | 8 +++++++-\n 1 file changed, 7 insertions(+), 1 deletion(-)\n\ndiff --git a/git-request-pull.sh b/git-request-pull.sh\nindex 5c1599752314..a23f03fddec0 100755\n--- a/git-request-pull.sh\n+++ b/git-request-pull.sh\n@@ -13,6 +13,7 @@ OPTIONS_STUCKLONG=\n OPTIONS_SPEC='git request-pull [options] start url [end]\n --\n p    show patch text as well\n+stat= specify stat output (see man git-diff for details)\n '\n \n . git-sh-setup\n@@ -21,11 +22,16 @@ GIT_PAGER=\n export GIT_PAGER\n \n patch=\n+stat=--stat\n while\tcase \"$#\" in 0) break ;; esac\n do\n \tcase \"$1\" in\n \t-p)\n \t\tpatch=-p ;;\n+\t--stat)\n+\t\tstat=\"$1=$2\"\n+\t\tshift\n+\t\t;;\n \t--)\n \t\tshift; break ;;\n \t-*)\n@@ -152,6 +158,6 @@ then\n fi &&\n \n git shortlog ^$baserev $headrev &&\n-git diff -M --stat --summary $patch $merge_base..$headrev || status=1\n+git diff -M $stat --summary $patch $merge_base..$headrev || status=1\n \n exit $status\n-- \n1.9.2\n"},{"id":"239578","messageId":"xmqqbnvqwrgu.fsf@gitster.dls.corp.google.com","threadId":"36489","inReplyTo":"1398331763-601-1-git-send-email-jslaby@suse.cz","subject":"Re: [PATCH] git-request-pull: add --stat option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-24T17:46:25Z","receivedAt":"2014-04-24T17:46:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jiri Slaby <jslaby@suse.cz> writes:\n\n> Which is passed on to git diff. I very need this option instead of\n> changing the terminal size.\n>\n> Signed-off-by: Jiri Slaby <jslaby@suse.cz>\n> ---\n\nInteresting.  I wonder if that suggests perhaps the default may be\nbetter if it were --stat=80 regardless of your terminal width.\n\nOh, wait.  That is the default we use when the output is not\nconnected to the terminal.\n\nInitially, I thought that the motivation behind this is that you got\ncomplaint from the recipient of your request that was generated to\nfit the width of _your_ taste (which is a lot wider than the\nstandard 80) because you run the command in a wide terminal.  But\nthat does not sound like it, as sending out a request would go more\nlike:\n\n    $ git request-pull ... >request.txt\n    $ edit request.txt\n    $ mua send request.txt\n\nand you would be getting 80-column output in that workflow.\n\nWhat are you using the output of the script for, and why do you\n\"very need this instead of changing the terminal size\"?\n\nI am puzzled.\n\nPerhaps is it the case that \"--stat\" output with full width of the\nterminal does *not* suit _your_ taste (not just the recipient's),\nand that is not limited to the request-pull output, but shared\nacross \"log -p --stat\", \"diff --stat\", and friends?  I wonder if it\nwould be a better solution for you and those in the same situation\nto set diff.statgraphwidth or something so that all these output are\nlimited to a reasonable width, if that is the case?\n\nPerhaps that diff.statgraphwidth that only specifies the graph part\nis too unwieldy and having a diff.statwidth or something that allows\nyou to customize that \"80 or terminal width\" in a more direct way is\nneeded?\n\nRegardless, having a way to pass thru an option, like this patch\ndoes, is independently a good thing, I would tend to think.  But \"I\nneed it instead of changing the terminal size\" does not look like a\nsufficient and readable justification that describes why.\n\n>  git-request-pull.sh | 8 +++++++-\n>  1 file changed, 7 insertions(+), 1 deletion(-)\n>\n> diff --git a/git-request-pull.sh b/git-request-pull.sh\n> index 5c1599752314..a23f03fddec0 100755\n> --- a/git-request-pull.sh\n> +++ b/git-request-pull.sh\n> @@ -13,6 +13,7 @@ OPTIONS_STUCKLONG=\n>  OPTIONS_SPEC='git request-pull [options] start url [end]\n>  --\n>  p    show patch text as well\n> +stat= specify stat output (see man git-diff for details)\n>  '\n>  \n>  . git-sh-setup\n> @@ -21,11 +22,16 @@ GIT_PAGER=\n>  export GIT_PAGER\n>  \n>  patch=\n> +stat=--stat\n>  while\tcase \"$#\" in 0) break ;; esac\n>  do\n>  \tcase \"$1\" in\n>  \t-p)\n>  \t\tpatch=-p ;;\n> +\t--stat)\n> +\t\tstat=\"$1=$2\"\n> +\t\tshift\n> +\t\t;;\n\nIf somebody did not want to give diffstat output for whatever\nreason, wouldn't it be natural to want to say\n\n\trequest-pull --stat= ...other options...\n\nrather than having to say it with an explicit empty string, i.e.\n\n\n\trequest-pull --stat \"\" ...other options...\n\nIn other words, I think the patch should also add\n\n\t--stat=*)\n        \tstat=\"$1\"\n\t\t;;\n\n>  \t--)\n>  \t\tshift; break ;;\n>  \t-*)\n> @@ -152,6 +158,6 @@ then\n>  fi &&\n>  \n>  git shortlog ^$baserev $headrev &&\n> -git diff -M --stat --summary $patch $merge_base..$headrev || status=1\n> +git diff -M $stat --summary $patch $merge_base..$headrev || status=1\n\nThis would not let the command notice a user error on the command\nline of request-pull, e.g.\n\n\trequest-pull --stat='30 bar baz' ...other options...\n\nbecause it will end up passing \"--stat=30\", \"bar\" and \"baz\" as\nseparate options to it, no?\n\n\tdiff -M ${stat+=\"$stat\"} ...\n\nperhaps?\n\n>  \n>  exit $status\n"}]}