Re: [PATCH 6/7] sh-setup: introduce require_clean_work_tree --quiet

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 23, 2013, 17:00 UTC
Message-ID: <7vvc7di3lp.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1366725724-1016-7-git-send-email-artagnon@gmail.com>

Ramkumar Ramachandra <artagnon@gmail.com> writes:

Show 5 quoted lines

> Some callers might want to know whether or not the worktree is clean,
> and require_clean_work_tree() has the logic for this.  The current
> implementation of the function prints a message and exits if the
> worktree wasn't clean.  Introduce a --quiet switch to get it to report
> the status of the worktree back to the caller.

This makes 75% sense, but I find it an extremely bad taste to tie exit vs return with --quiet. They are orthogonal and unrelated concepts.

Doing this

	if (require_clean_work_tree) 2>/dev/null
        then
		: happy, the working tree is clean
	else
		... let's stash ...
	fi

without changing anything else may be better than adding such a conflating --quiet option.

Show 42 quoted lines

> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
>  git-sh-setup.sh | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/git-sh-setup.sh b/git-sh-setup.sh
> index 2f78359..5fa22a8 100644
> --- a/git-sh-setup.sh
> +++ b/git-sh-setup.sh
> @@ -172,6 +172,7 @@ require_clean_work_tree () {
>  
>  	if ! git diff-files --quiet --ignore-submodules
>  	then
> +		test "$1" = "--quiet" ||
>  		echo >&2 "Cannot $1: You have unstaged changes."
>  		err=1
>  	fi
> @@ -180,9 +181,11 @@ require_clean_work_tree () {
>  	then
>  		if [ $err = 0 ]
>  		then
> -		    echo >&2 "Cannot $1: Your index contains uncommitted changes."
> +			test "$1" = "--quiet" ||
> +			echo >&2 "Cannot $1: Your index contains uncommitted changes."
>  		else
> -		    echo >&2 "Additionally, your index contains uncommitted changes."
> +			test "$1" = "--quiet" ||
> +			echo >&2 "Additionally, your index contains uncommitted changes."
>  		fi
>  		err=1
>  	fi
> @@ -190,8 +193,9 @@ require_clean_work_tree () {
>  	if [ $err = 1 ]
>  	then
>  		test -n "$2" && echo >&2 "$2"
> -		exit 1
> +		test "$1" = "--quiet" || exit 1
>  	fi
> +	return $err
>  }
>  
>  # Generate a sed script to parse identities from a commit.