David Aguilar <davvid@xxxxxxxxx> writes:
> Remove return statements and rework check_unchanged() so that the exit
> status from the last evaluated expression bubbles up to the callers.
>
> Signed-off-by: David Aguilar <davvid@xxxxxxxxx>
> ---
> git-mergetool--lib.sh | 20 +++++---------------
> 1 file changed, 5 insertions(+), 15 deletions(-)
>
> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> index 2b66351..fe61e89 100644
> --- a/git-mergetool--lib.sh
> +++ b/git-mergetool--lib.sh
> @@ -92,7 +92,7 @@ translate_merge_tool_path () {
> check_unchanged () {
> if test "$MERGED" -nt "$BACKUP"
> then
> - status=0
> + return 0
> else
> while true
> do
> @@ -100,8 +100,8 @@ check_unchanged () {
> printf "Was the merge successful? [y/n] "
> read answer || return 1
> case "$answer" in
> - y*|Y*) status=0; break ;;
> - n*|N*) status=1; break ;;
> + y*|Y*) return 0 ;;
> + n*|N*) return 1 ;;
> esac
> done
> fi
Note: The above left in the response not because I have any comment
on or objection to it but because it is relevant to the comment on
the next hunk.
> @@ -130,13 +128,10 @@ setup_user_tool () {
> then
> touch "$BACKUP"
> ( eval $merge_tool_cmd )
> - status=$?
> check_unchanged
> else
> ( eval $merge_tool_cmd )
> - status=$?
> fi
> - return $status
> }
> }
The caller of this funciton used to get the status from running
$merge_tool_cmd, but now it gets the result from check_unchanged.
Maybe that is more correct thing to report, but this does change the
behaviour, no?
... goes and looks ...
Ahh, the assignment to $status before running check_unchanged was not
doing anything useful, because check_unchanged stomped on $status
before it returned anyway.
So the net effect of this hunk to the caller's is unchanged. It is
a bit tricky but the end result looks correct.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html