[m-rev.] for review: git hook improvements

[Peter Wang]

On Wed, 13 Mar 2024 16:21:51 +1100 "Zoltan Somogyi" <zoltan.somogyi at runbox.com> wrote:
> 
> On 2024-03-13 15:57 +11:00 AEDT, "Peter Wang" <novalazy at gmail.com> wrote:
> > On Sat, 09 Mar 2024 18:34:58 +1100 "Zoltan Somogyi" <zoltan.somogyi at runbox.com> wrote:
> >> For review by Peter.
> 
> >>  rootdir=$( git rev-parse --show-toplevel )
> >> -update_copyright="$rootdir/git_hooks/update_copyright"
> >> +if test "${UPDATE_MERCURY_COPYRIGHT}" != ""
> >> +then
> >> +    update_copyright="${UPDATE_MERCURY_COPYRIGHT}"
> >> +else
> >> +    update_copyright="$rootdir/git_hooks/update_copyright"
> >> +fi
> > 
> > Obviously that could also be:
> > 
> >     update_copyright=${UPDATE_MERCURY_COPYRIGHT-"$rootdir/git_hooks/update_copyright"}
> 
> I could be, but there about a dozen short char sequences you can put
> between the var name and the default, they have all slightly different semantics,
> and I find it hard to remember which is which. The longer code I used
> is easier to check, at least for fossils like myself.
> 

I thought that might be the case.

I've started writing shell scripts with 'set -u' by default,
so now I am always suspicious of expansions of possibly-unset variables,
and got more familiar with ${var:-} and ${var-}, but I still need to
look up which is which.

This particular script doesn't use 'set -u'. There's probably a reason
for that, but I don't remember it just now.

> I followed all your other suggestions. The attached diff includes them all,
> as well as the previously missing Makefile. Does it have your approval?

Yes, it's fine.

>  rootdir=$( git rev-parse --show-toplevel )
> -update_copyright="$rootdir/git_hooks/update_copyright"
> -
> -# Only continue if the update_copyright program has been compiled.
> -if test ! -x "$update_copyright"
> +if test "${UPDATE_MERCURY_COPYRIGHT}" != ""
>  then
> -    exit
> +    update_copyright="${UPDATE_MERCURY_COPYRIGHT}"
> +else
> +    update_copyright="${rootdir}/git_hooks/update_copyright"
>  fi
>  
> +# Continue only if the update_copyright program is available.
> +command -v ${update_copyright} > /dev/null || exit 0

Add quotes around that.

Peter