Subject: Re: [PATCH] Make stashing nothing exit 1

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Sat, 23 Mar 2019 08:54:37 +0100

On Sat, Mar 23 2019, Keith Smiley wrote:

> In the case there are no files to stash, but the user asked to stash, we
> should exit 1 since the stashing failed.
> ---
>  git-stash.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git-stash.sh b/git-stash.sh
> index 789ce2f41d4a3..ca362b1a31277 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -318,7 +318,7 @@ push_stash () {
>  	if no_changes "$@"
>  	then
>  		say "$(gettext "No local changes to save")"
> -		exit 0
> +		exit 1
>  	fi
>
>  	git reflog exists $ref_stash ||

Thanks for contributing, some points:

 * stash is currently (in the 'next' branch) being rewritten in C. It's
   a better move at this point to patch that version, this code is about
   to be deleted.

 * This is missing a corresponding test, and skimming the stash manpage
   we should document how these exit codes are supposed to act.

 * Shouldn't we do this consistently across all the other sub-commands?
   Trying some of them seems 'push' may be the odd one out, but maybe
   I've missed some (and this would/should be covered by
   tests). I.e. some single test that does a bunch of ops with no
   entries / nothing to stash and asserts exit codes.