[PATCH] git-request-pull: add --stat option

[PATCH] git-request-pull: add --stat option

[Jiri Slaby]

Which is passed on to git diff. I very need this option instead of
changing the terminal size.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 git-request-pull.sh | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/git-request-pull.sh b/git-request-pull.sh
index 5c1599752314..a23f03fddec0 100755
--- a/git-request-pull.sh
+++ b/git-request-pull.sh
@@ -13,6 +13,7 @@ OPTIONS_STUCKLONG=
 OPTIONS_SPEC='git request-pull [options] start url [end]
 --
 p    show patch text as well
+stat= specify stat output (see man git-diff for details)
 '
 
 . git-sh-setup
@@ -21,11 +22,16 @@ GIT_PAGER=
 export GIT_PAGER
 
 patch=
+stat=--stat
 while	case "$#" in 0) break ;; esac
 do
 	case "$1" in
 	-p)
 		patch=-p ;;
+	--stat)
+		stat="$1=$2"
+		shift
+		;;
 	--)
 		shift; break ;;
 	-*)
@@ -152,6 +158,6 @@ then
 fi &&
 
 git shortlog ^$baserev $headrev &&
-git diff -M --stat --summary $patch $merge_base..$headrev || status=1
+git diff -M $stat --summary $patch $merge_base..$headrev || status=1
 
 exit $status
-- 
1.9.2

Re: [PATCH] git-request-pull: add --stat option

[Junio C Hamano]

Jiri Slaby <jslaby@suse.cz> writes:

> Which is passed on to git diff. I very need this option instead of
> changing the terminal size.
>
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> ---

Interesting.  I wonder if that suggests perhaps the default may be
better if it were --stat=80 regardless of your terminal width.

Oh, wait.  That is the default we use when the output is not
connected to the terminal.

Initially, I thought that the motivation behind this is that you got
complaint from the recipient of your request that was generated to
fit the width of _your_ taste (which is a lot wider than the
standard 80) because you run the command in a wide terminal.  But
that does not sound like it, as sending out a request would go more
like:

    $ git request-pull ... >request.txt
    $ edit request.txt
    $ mua send request.txt

and you would be getting 80-column output in that workflow.

What are you using the output of the script for, and why do you
"very need this instead of changing the terminal size"?

I am puzzled.

Perhaps is it the case that "--stat" output with full width of the
terminal does *not* suit _your_ taste (not just the recipient's),
and that is not limited to the request-pull output, but shared
across "log -p --stat", "diff --stat", and friends?  I wonder if it
would be a better solution for you and those in the same situation
to set diff.statgraphwidth or something so that all these output are
limited to a reasonable width, if that is the case?

Perhaps that diff.statgraphwidth that only specifies the graph part
is too unwieldy and having a diff.statwidth or something that allows
you to customize that "80 or terminal width" in a more direct way is
needed?

Regardless, having a way to pass thru an option, like this patch
does, is independently a good thing, I would tend to think.  But "I
need it instead of changing the terminal size" does not look like a
sufficient and readable justification that describes why.

>  git-request-pull.sh | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/git-request-pull.sh b/git-request-pull.sh
> index 5c1599752314..a23f03fddec0 100755
> --- a/git-request-pull.sh
> +++ b/git-request-pull.sh
> @@ -13,6 +13,7 @@ OPTIONS_STUCKLONG=
>  OPTIONS_SPEC='git request-pull [options] start url [end]
>  --
>  p    show patch text as well
> +stat= specify stat output (see man git-diff for details)
>  '
>  
>  . git-sh-setup
> @@ -21,11 +22,16 @@ GIT_PAGER=
>  export GIT_PAGER
>  
>  patch=
> +stat=--stat
>  while	case "$#" in 0) break ;; esac
>  do
>  	case "$1" in
>  	-p)
>  		patch=-p ;;
> +	--stat)
> +		stat="$1=$2"
> +		shift
> +		;;

If somebody did not want to give diffstat output for whatever
reason, wouldn't it be natural to want to say

	request-pull --stat= ...other options...

rather than having to say it with an explicit empty string, i.e.


	request-pull --stat "" ...other options...

In other words, I think the patch should also add

	--stat=*)
        	stat="$1"
		;;

>  	--)
>  		shift; break ;;
>  	-*)
> @@ -152,6 +158,6 @@ then
>  fi &&
>  
>  git shortlog ^$baserev $headrev &&
> -git diff -M --stat --summary $patch $merge_base..$headrev || status=1
> +git diff -M $stat --summary $patch $merge_base..$headrev || status=1

This would not let the command notice a user error on the command
line of request-pull, e.g.

	request-pull --stat='30 bar baz' ...other options...

because it will end up passing "--stat=30", "bar" and "baz" as
separate options to it, no?

	diff -M ${stat+="$stat"} ...

perhaps?

>  
>  exit $status