Phillip,

Hello again! I have adopted your recommendations everywhere except for 
`git checkout @{-1}`. Details below.

On 9/8/25 10:21, Phillip Wood wrote:

On 05/09/2025 03:27, Colin Stagner wrote:

quoted

+    while read -r trailer val
+    do
+        case "$trailer" in
+        (git-subtree-dir:)
+            subtree_dir="${val%/}" ;;
+        (git-subtree-mainline:)
+            have_mainline=y ;;
+        esac
+    done

We do not use the optional '(' in case statements

Will fix in v3.

quoted

-    if test -n "$(git log -1 --grep="git-subtree-dir:" $rev)"
+    if test -n "${subtree_dir:-}" &&
+        test -z "${have_mainline:-}" &&
+        test "${subtree_dir}" != "$arg_prefix"

What's the idea behind using "${var:-}" rather than "{var}"?

I write a lot of shell scripts that run "set -u" (aka "set -o nounset"), 
so I do this a lot when testing for empty vars. In this case, it's not 
actually necessary since `have_mainline` is explicitly defined above. 
And we don't run `set -u` anyway.

Will remove from v3.

quoted

+test_create_subtree_add () {
+    (
+        cd "$1" &&
+        orphan="$2" &&
+        prefix="$3" &&
+        filename="$4" &&
+        shift 4 &&
+        last="$(git branch --show-current)" &&
+        git checkout --orphan "$orphan" &&
+        git rm -rf . &&

If you use "git switch --orphan" that clears the worktree for you

Very useful. I'll start using it in v3.

quoted

+        test_commit "$filename" &&
+        git checkout "$last" &&

I think this could be "git checkout @{-1}" and then we'd avoid having to 
run "git branch" above

I experimented with this, but I couldn't get it to work on git 2.44. 
Although the reflog shows the refs I expect, using

     git switch '@{-1}'

dies with

     fatal: invalid reference: @{-1}

checkout doesn't work either. Perhaps there is something about --orphan 
that is messing up the history.

I could make `test_create_subtree_add` take a mainline branch name, 
but... unless there's something unsound about v2, I think we should just 
keep v2. `git branch --show-current` looks like well-defined porcelain.

Any other ideas?

quoted

+# The test covers:
+# - An initial `subtree add`; and
+# - A follow-up `subtree merge`
+# both with and without `--squashed`.
+for is_squashed in '' 'y';

no need for ';' at the end of the line

Fixed for v3.

quoted

+        subtree_test_create_repo "$test_count" &&
+        (
+            cd "$test_count" &&
+            mkdir subA &&
+            test_commit subA/file1 &&
+            git branch -m main &&

For tests that depend on the default branch name you can add

     GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
     export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME

to the start of the file before it sources test-lib.sh.

GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main looks very common, so I'll go 
with that for v3.

quoted

+            test_create_subtree_add \
+                . mksubtree subA/subB file2 ${is_squashed:+--squash} &&
+            test -e subA/file1.t &&

We have test_path_is_file() for this which prints a useful diagnostic 
message

Fixed all occurrences in v3.


Colin