Skip to content

Commit f8e90b9

Browse files
Colin Stagnergitster
authored andcommitted
contrib/subtree: reduce function side-effects
`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> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent 628a66c commit f8e90b9

File tree

1 file changed

+7
-2
lines changed

1 file changed

+7
-2
lines changed

contrib/subtree/git-subtree.sh

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,10 @@ try_remove_previous () {
373373
}
374374

375375
# Usage: process_subtree_split_trailer SPLIT_HASH MAIN_HASH [REPOSITORY]
376+
#
377+
# Parse SPLIT_HASH as a commit. If the commit is not found, fetches
378+
# REPOSITORY and tries again. If found, prints full commit hash.
379+
# Otherwise, dies.
376380
process_subtree_split_trailer () {
377381
assert test $# -ge 2
378382
assert test $# -le 3
@@ -400,6 +404,7 @@ process_subtree_split_trailer () {
400404
die "$fail_msg"
401405
fi
402406
fi
407+
echo "${sub}"
403408
}
404409

405410
# Usage: find_latest_squash DIR [REPOSITORY]
@@ -432,7 +437,7 @@ find_latest_squash () {
432437
main="$b"
433438
;;
434439
git-subtree-split:)
435-
process_subtree_split_trailer "$b" "$sq" "$repository"
440+
sub="$(process_subtree_split_trailer "$b" "$sq" "$repository")" || exit 1
436441
;;
437442
END)
438443
if test -n "$sub"
@@ -489,7 +494,7 @@ find_existing_splits () {
489494
main="$b"
490495
;;
491496
git-subtree-split:)
492-
process_subtree_split_trailer "$b" "$sq" "$repository"
497+
sub="$(process_subtree_split_trailer "$b" "$sq" "$repository")" || exit 1
493498
;;
494499
END)
495500
debug "Main is: '$main'"

0 commit comments

Comments
 (0)