Re: [PATCH v7 0/4] worktree: Support `--orphan` when creating new worktrees

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Sat, Jan 07 2023, Jacob Abel wrote:

> While working with the worktree based git workflow, I realised that setting
> up a new git repository required switching between the traditional and
> worktree based workflows. Searching online I found a SO answer [1] which
> seemed to support this and which indicated that adding support for this should
> not be technically difficult.
>
> This patchset has four parts:
>   * adding `-B` to the usage docs (noticed during dev and it seemed too small
>     to justify a separate submission)
>   * adding a helper fn to simplify testing for mutual exclusion of options
>     in `t/t2400-worktree-add.sh`
>   * adding orphan branch functionality (as is present in `git-switch`)
>     to `git-worktree-add`
>   * adding an advise for using --orphan when `git worktree add` fails due to
>     a bad ref.
>
> Changes from v6:
>   * Removed helper save_param_arr() introduced in v6 from t2400 (2/4) [2].
>   * Reverted changes introduced in v6 to test_wt_add_excl() from t2400 (2/4) [3].
>   * Changed test_wt_add_excl() to use `local opts="$*"` (2/4) [3].
>   * Added check to test_wt_add_excl() to better validate test results (2/4).
>   * Re-add &&-chains to test_wt_add_excl() (2/4) [4].
>   * Reverted changes introduced in v6 to test_wt_add_empty_repo_orphan_hint()
>     from t2400 (4/4) [3].
>   * Changed test_wt_add_empty_repo_orphan_hint() to use `local opts="$*"` (4/4) [3].
>   * Added check to test_wt_add_empty_repo_orphan_hint() to better validate test
>     results (4/4).
>   * Re-add &&-chains to test_wt_add_empty_repo_orphan_hint() (2/4) [4].

This round looks good to me, except for a small tiny (and new)
portability issue that needs fixing:

> Range-diff against v6:
> 1:  a9ef3eca7b = 1:  a9ef3eca7b worktree add: include -B in usage docs
> 2:  c03c112f79 ! 2:  d124cc481c worktree add: refactor opt exclusion tests
>     @@ t/t2400-worktree-add.sh: test_expect_success '"add" no auto-vivify with --detach
>      -test_expect_success '"add" -b/-B mutually exclusive' '
>      -	test_must_fail git worktree add -b poodle -B poodle bamboo main
>      -'
>     -+# Saves parameter sequence/array as a string so they can be safely stored in a
>     -+# variable and restored with `eval "set -- $arr"`. Sourced from
>     -+# https://stackoverflow.com/a/27503158/15064705
>     -+save_param_arr () {
>     -+	local i
>     -+	for i;
>     -+	do
>     -+		# For each argument:
>     -+		# 1. Append "\n" after each entry
>     -+		# 2. Convert "'" into "'\''"
>     -+		# 3. Prepend "'" before each entry
>     -+		# 4. Append " \" after each entry
>     -+		printf "%s\\n" "$i" | sed "
>     -+			s/'/'\\\\''/g
>     -+			1s/^/'/
>     -+			\$s/\$/' \\\\/
>     -+		"
>     -+	done
>     -+	echo " "
>     -+}
>     -
>     +-
>      -test_expect_success '"add" -b/--detach mutually exclusive' '
>      -	test_must_fail git worktree add -b poodle --detach bamboo main
>      -'

Good to get rid of this.

>      +# Helper function to test mutually exclusive options.
>     ++#
>     ++# Note: Quoted arguments containing spaces are not supported.
>      +test_wt_add_excl () {
>     -+	local arr=$(save_param_arr "$@")
>     -+	test_expect_success "'worktree add' with $* has mutually exclusive options" '
>     -+		eval "set -- $arr" &&
>     -+		test_must_fail git worktree add "$@"
>     ++	local opts="$*" &&
>     ++	test_expect_success "'worktree add' with '$opts' has mutually exclusive options" '
>     ++		test_must_fail git worktree add $opts 2>actual &&
>     ++		grep -P "fatal:( options)? .* cannot be used together" actual

This is the new unportable code, the "-P" option is specific to GNU
grep, and here you're relying on the "?" syntax along with other
ERE-like syntax.

It looks like the minimal fix here is to just change -P to -E, which we
can use, you're not using any PCRE-syntax, but just syntax that's part
of ERE.