[PATCH v2 1/3] contrib/subtree: reduce function side-effects
- From
- Colin Stagner <ask+git@howdoi.land>
- Date
- Mar 5, 2026, 23:55 UTC
- Message-ID
- <20260305-cs-subtree-split-recursion-v2-1-7266be870ba9@howdoi.land>
- In-Reply-To
- <20260305-cs-subtree-split-recursion-v2-0-7266be870ba9@howdoi.land>
`process_subtree_split_trailer()` communicates its return value to the caller by setting a variable (`sub`) that is also defined by the calling function. This is both unclear and encourages side-effects.
Invoke this function in a sub-shell instead.
Signed-off-by: Colin Stagner <ask+git@howdoi.land> --- contrib/subtree/git-subtree.sh | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh index 791fd8260c..bae5d9170b 100755 --- a/contrib/subtree/git-subtree.sh +++ b/contrib/subtree/git-subtree.sh @@ -373,6 +373,10 @@ try_remove_previous () { } # Usage: process_subtree_split_trailer SPLIT_HASH MAIN_HASH [REPOSITORY] +# +# Parse SPLIT_HASH as a commit. If the commit is not found, fetches +# REPOSITORY and tries again. If found, prints full commit hash. +# Otherwise, dies. process_subtree_split_trailer () { assert test $# -ge 2 assert test $# -le 3 @@ -400,6 +404,7 @@ process_subtree_split_trailer () { die "$fail_msg" fi fi + echo "${sub}" } # Usage: find_latest_squash DIR [REPOSITORY] @@ -432,7 +437,7 @@ find_latest_squash () { main="$b" ;; git-subtree-split:) - process_subtree_split_trailer "$b" "$sq" "$repository" + sub="$(process_subtree_split_trailer "$b" "$sq" "$repository")" || exit 1 ;; END) if test -n "$sub" @@ -489,7 +494,7 @@ find_existing_splits () { main="$b" ;; git-subtree-split:) - process_subtree_split_trailer "$b" "$sq" "$repository" + sub="$(process_subtree_split_trailer "$b" "$sq" "$repository")" || exit 1 ;; END) debug "Main is: '$main'"
-- 2.43.0