On March 24, 2022 3:00 PM, Junio C Hamano wrote:
>Subject: Re: [PATCH v8 21/30] t7527: create test for fsmonitor--daemon
>
>"Jeff Hostetler via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
>
>> From: Jeff Hostetler <jeffhost@xxxxxxxxxxxxx>
>>
>> Signed-off-by: Jeff Hostetler <jeffhost@xxxxxxxxxxxxx>
>> Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
>
>I hadn't signed off on this one yet ;-)
>
>> +is_value () {
>> + test -n "$1" && test "${1::1}" != "-"
>> +}
>
>${var:ofs:len} is a bash-ism. If you run this test under /bin/dash instead of
>/bin/dash, you'll likely see it fail.
>
>If it were a good idea to see if $1 begins with a dash, a more natural (to shell
>programmers) way to do so is
>
> case "$1" in -*) false ;; ?*) true ;; *) false ;; esac
>
>but given how this is used below, we do not want to special case dash.
>
>There isn't anything wrong in "mkdir ./-foo && start_daemon -C -foo"
>in other words.
>
>> +start_daemon () {
>> + r= &&
>> + tf= &&
>> + t2= &&
>> + tk= &&
>
>FYI, you can write these on a single line, i.e.
>
> r= tf= t2= tk= &&
>
>Spending lines and spaces for the meat of the script would enhance readability but
>for things like a boilerplate "we clear variables before using them", being concise
>may be less distracting.
>
>> + while test "$#" -ne 0
>> + do
>> + case "$1" in
>> + -C)
>> + shift;
>> + is_value $1 || BUG "error: -C requires value"
>> + r="-C $1"
>> + shift
>> + ;;
>> + ...
>> + esac
>> + done &&
>
>A more natural way to write these loops is
>
> while ...
> do
> case "$1" in
> -C)
> r="-C ${2?}"
> shift
> ;;
> ... all other options you handle ...
> -*)
> echo >&2 "unknown option $1"
> exit 1
> ;;
> *)
> break
> ;;
> esac
> shift
> done
>
>i.e. shifting out what we just saw is the default and happens immediately after the
>case/esac, and extra shift after consuming an option parameter happens in each
>case arm.
>
>An acceptable slight variation is
>
> -C)
> shift
> r="-C ${1?}"
> ;;
>
>but the first form is more logical and clear, i.e. "when we see '-C', we want two on
>the command line, -C itself and the parameter it takes"
>is conveyed more strongly with "${2?}" there.
>
>For an additional bonus, we could also accept the stuck form, i.e.
>
> case "$1" in
> -C)
> r="-C ${2?}"
> shift
> ;;
> -C*)
> r="-C {$1#-C}"
> ;;
> ...
May I request a bit of extra time on the -rc0 to -rc1 cycle for this? I have a feeling that while testing this is probably going to go well, I would like to have a bit of extra time for anything that might come up. There are a lot of moving parts to this series. Not being critical, but debugging scripts on my platforms can be a bit rough at times.
Thanks,
Randall
