Re: [GSoC][PATCH v7 14/26] stash: convert store to builtin

[Thomas Gummerer <t.gummerer@gmail.com>]

On 08/08, Paul-Sebastian Ungureanu wrote:
> Add stash store to the helper and delete the store_stash function
> from the shell script.
> 
> Add the usage string which was forgotten in the shell script.

I think similarly to 'git stash create', which also doesn't appear in
the usage, this was intentionally omitted in the shell script.  The
reason for the omission is that this is only intended to be useful in
scripts, and not in interactive usage.  As such it doesn't add much
value in showing it in 'git stash -h'.  Meanwhile it is in the
synopsis in the man page.

If we want to add it to the help output, I think it would be best to
do so in a separate commit, and for 'git stash create' as well.  But
I'm not sure that's a good change.

> Signed-off-by: Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com>
> ---
>  builtin/stash--helper.c | 52 +++++++++++++++++++++++++++++++++++++++++
>  git-stash.sh            | 43 ++--------------------------------
>  2 files changed, 54 insertions(+), 41 deletions(-)
> 
> diff --git a/builtin/stash--helper.c b/builtin/stash--helper.c
> index ec8c38c6f..5ff810f8c 100644
> --- a/builtin/stash--helper.c
> +++ b/builtin/stash--helper.c
> @@ -20,6 +20,7 @@ static const char * const git_stash_helper_usage[] = {
>  	N_("git stash--helper ( pop | apply ) [--index] [-q|--quiet] [<stash>]"),
>  	N_("git stash--helper branch <branchname> [<stash>]"),
>  	N_("git stash--helper clear"),
> +	N_("git stash--helper store [-m|--message <message>] [-q|--quiet] <commit>"),
>  	NULL
>  };
>  
> @@ -58,6 +59,11 @@ static const char * const git_stash_helper_clear_usage[] = {
>  	NULL
>  };
>  
> +static const char * const git_stash_helper_store_usage[] = {
> +	N_("git stash--helper store [-m|--message <message>] [-q|--quiet] <commit>"),
> +	NULL
> +};
> +
>  static const char *ref_stash = "refs/stash";
>  static int quiet;
>  static struct strbuf stash_index_path = STRBUF_INIT;
> @@ -731,6 +737,50 @@ static int show_stash(int argc, const char **argv, const char *prefix)
>  	return 0;
>  }
>  
> +static int do_store_stash(const char *w_commit, const char *stash_msg,
> +			  int quiet)
> +{
> +	int ret = 0;
> +	struct object_id obj;
> +
> +	if (!stash_msg)
> +		stash_msg  = xstrdup("Created via \"git stash--helper store\".");

I assume we're going to s/--helper// in a later commit?  Not sure
adding the '--helper' here is necessary, as a user would never invoke
'git stash--helper' directly, so they would expect the stash to be
created by 'git stash store'.  Anyway that's fairly minor, as I assume
this is going to change by the end of the patch series.

> +
> +	ret = get_oid(w_commit, &obj);
> +	if (!ret) {
> +		ret = update_ref(stash_msg, ref_stash, &obj, NULL,
> +				 REF_FORCE_CREATE_REFLOG,
> +				 quiet ? UPDATE_REFS_QUIET_ON_ERR :
> +				 UPDATE_REFS_MSG_ON_ERR);
> +	}
> +	if (ret && !quiet)
> +		fprintf_ln(stderr, _("Cannot update %s with %s"),
> +			   ref_stash, w_commit);
> +
> +	return ret;
> +}
> +
> +static int store_stash(int argc, const char **argv, const char *prefix)
> +{
> +	const char *stash_msg = NULL;
> +	struct option options[] = {
> +		OPT__QUIET(&quiet, N_("be quiet, only report errors")),
> +		OPT_STRING('m', "message", &stash_msg, "message", N_("stash message")),
> +		OPT_END()
> +	};
> +
> +	argc = parse_options(argc, argv, prefix, options,
> +			     git_stash_helper_store_usage,
> +			     PARSE_OPT_KEEP_UNKNOWN);
> +
> +	if (argc != 1) {
> +		fprintf(stderr, _("\"git stash--helper store\" requires one <commit> argument\n"));
> +		return -1;
> +	}
> +
> +	return do_store_stash(argv[0], stash_msg, quiet);
> +}
> +
>  int cmd_stash__helper(int argc, const char **argv, const char *prefix)
>  {
>  	pid_t pid = getpid();
> @@ -765,6 +815,8 @@ int cmd_stash__helper(int argc, const char **argv, const char *prefix)
>  		return !!list_stash(argc, argv, prefix);
>  	else if (!strcmp(argv[0], "show"))
>  		return !!show_stash(argc, argv, prefix);
> +	else if (!strcmp(argv[0], "store"))
> +		return !!store_stash(argc, argv, prefix);
>  
>  	usage_msg_opt(xstrfmt(_("unknown subcommand: %s"), argv[0]),
>  		      git_stash_helper_usage, options);
> diff --git a/git-stash.sh b/git-stash.sh
> index 0d05cbc1e..5739c5152 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -191,45 +191,6 @@ create_stash () {
>  	die "$(gettext "Cannot record working tree state")"
>  }
>  
> -store_stash () {
> -	while test $# != 0
> -	do
> -		case "$1" in
> -		-m|--message)
> -			shift
> -			stash_msg="$1"
> -			;;
> -		-m*)
> -			stash_msg=${1#-m}
> -			;;
> -		--message=*)
> -			stash_msg=${1#--message=}
> -			;;
> -		-q|--quiet)
> -			quiet=t
> -			;;
> -		*)
> -			break
> -			;;
> -		esac
> -		shift
> -	done
> -	test $# = 1 ||
> -	die "$(eval_gettext "\"$dashless store\" requires one <commit> argument")"
> -
> -	w_commit="$1"
> -	if test -z "$stash_msg"
> -	then
> -		stash_msg="Created via \"git stash store\"."
> -	fi
> -
> -	git update-ref --create-reflog -m "$stash_msg" $ref_stash $w_commit
> -	ret=$?
> -	test $ret != 0 && test -z "$quiet" &&
> -	die "$(eval_gettext "Cannot update \$ref_stash with \$w_commit")"
> -	return $ret
> -}
> -
>  push_stash () {
>  	keep_index=
>  	patch_mode=
> @@ -308,7 +269,7 @@ push_stash () {
>  		clear_stash || die "$(gettext "Cannot initialize stash")"
>  
>  	create_stash -m "$stash_msg" -u "$untracked" -- "$@"
> -	store_stash -m "$stash_msg" -q $w_commit ||
> +	git stash--helper store -m "$stash_msg" -q $w_commit ||
>  	die "$(gettext "Cannot save the current status")"
>  	say "$(eval_gettext "Saved working directory and index state \$stash_msg")"
>  
> @@ -468,7 +429,7 @@ create)
>  	;;
>  store)
>  	shift
> -	store_stash "$@"
> +	git stash--helper store "$@"
>  	;;
>  drop)
>  	shift
> -- 
> 2.18.0.573.g56500d98f
>