Hi,
On Fri, Aug 28, 2015 at 12:20 AM, SZEDER Gábor <szeder@xxxxxxxxxx> wrote:
> Hi,
>
> I haven't made up my mind about this feature yet, but have a few
> comments about its implementation.
Thanks for taking your time!
>
>> diff --git a/git-stash.sh b/git-stash.sh
>> index 1d5ba7a..8432435 100755
>> --- a/git-stash.sh
>> +++ b/git-stash.sh
>> @@ -33,6 +33,12 @@ else
>> reset_color=
>> fi
>>
>> +if git config --get stash.showflag > /dev/null 2> /dev/null; then
>> + show_flag=$(git config --get stash.showflag)
>> +else
>> + show_flag=--stat
>> +fi
>> +
>
> Forking and executing processes are costly on some important platforms
> we care about, so we should strive to avoid them whenever possible.
>
> - This hunk runs the the exact same 'git config' command twice. Run it
> only once, perhaps something like this:
>
> show_flag=$(git config --get stash.showflag || echo --stat)
>
> (I hope there are no obscure crazy 'echo' implemtations out there
> that might barf on the unknown option '--stat'...)
What about `echo "--stat"` then?
>
> - It runs 'git config' in the main code path, i.e. even for subcommands
> other than 'show'. Run it only for 'git stash show'.
>
> - This config setting is not relevant if there were options given on the
> command line. Run it only if there are no options given, i.e. when
> $FLAGS is empty.
Fair enough. I'll resend v2.
Thanks,
Namhyung
>
>
>> no_changes () {
>> git diff-index --quiet --cached HEAD --ignore-submodules -- &&
>> git diff-files --quiet --ignore-submodules &&
>> @@ -305,7 +311,7 @@ show_stash () {
>> ALLOW_UNKNOWN_FLAGS=t
>> assert_stash_like "$@"
>>
>> - git diff ${FLAGS:---stat} $b_commit $w_commit
>> + git diff ${FLAGS:-${show_flag}} $b_commit $w_commit
>> }
>>
>> show_help () {
>> --
>> 2.5.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html