From: Colin Stagner Date: Sun, 15 Feb 2026 20:17:43 GMT Subject: [PATCH 1/3] contrib/subtree: reduce function side-effects Message-ID: <20260215201748.889866-2-ask+git@howdoi.land> In-Reply-To: <20260215201748.889866-1-ask+git@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 --- 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 17106d1a72..1cdf39a481 100755 --- a/contrib/subtree/git-subtree.sh +++ b/contrib/subtree/git-subtree.sh @@ -370,6 +370,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 @@ -397,6 +401,7 @@ process_subtree_split_trailer () { die "$fail_msg" fi fi + echo "${sub}" } # Usage: find_latest_squash DIR [REPOSITORY] @@ -429,7 +434,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" @@ -486,7 +491,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