Hi Junio,
On Thu, 30 Aug 2018, Junio C Hamano wrote:
> Paul-Sebastian Ungureanu <ungureanupaulsebastian@xxxxxxxxx> writes:
> > +test_stat_and_patch () {
> > + if test "<unset>" = "$1"
> > + then
> > + test_might_fail git config --unset stash.showStat
> > + else
> > + test_config stash.showStat "$1"
> > + fi &&
> > +
> > + if test "<unset>" = "$2"
> > + then
> > + test_might_fail git config --unset stash.showPatch
>
> I think you are trying to protect yourself from an error triggered
> by unsetting what is not set, but for that, test_unconfig is
> probably a better choice, as it still catches errors of other types
> and ignores only that "unset a variable that is not set" error.
My mistake. I did not realize that tehere is a `test_unconfig` when I
suggested the current version.
> > + shift &&
> > + shift &&
>
> You can use "shift 2 &&" here (not worth a reroll).
Again, my mistake. I was not sure how portable that construct is. (Is it?)
>
> > + echo 2 >file.t &&
>
> > + git diff "$@" >expect &&
>
> When the caller does not give $3 to this function, it does not look
> at 'expect'. I think it is clearer if you did
>
> if test $# != 0
> then
> git diff "$@" >expect
> fi &&
>
> here, and ...
>
> > + git stash &&
> > + git stash show >actual &&
> > +
> > + if test -z "$1"
>
> ... wrote this as
>
> if test $# = 0
>
> The only difference between '-z "$1"' and '$# = 0' is when he caller
> passes an empty string to the function as $3, which you never do, so
> the distinction is theoretical, but using $# makes your intention
> clear that you do not mean to treat an empty string any specially.
Good advice in general (in this case, we know the callers).
Paul, when it comes to shell scripting, listen to Junio more than myself
because he is even more of a shell script wiz than I am.
Ciao,
Dscho
