Re: [PATCH v8 21/30] t7527: create test for fsmonitor--daemon

"Jeff Hostetler via GitGitGadget" <gitgitgadget@gmail.com> writes:

I hadn't signed off on this one yet ;-)

${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.

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.

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}"
			;;
		...