Re: [PATCH] git-request-pull: add --stat option
Jiri Slaby <jslaby@suse.cz> writes:
Show 5 quoted lines
> 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.txtand 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.
Show 29 quoted lines
> 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"
;;Show 9 quoted lines
> --)
> 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
