From: Thomas Gummerer <t.gummerer@gmail.com>
Date: Wed, 15 Aug 2018 19:41:20 GMT
Subject: Re: [GSoC][PATCH v7 09/26] stash: implement the "list" command in the builtin
Message-ID: <20180815194120.GK2734@hank.intra.tgummerer.com>
In-Reply-To: <47556d40a9944e8cc45ba3df8e12c80a1898b160.1533753605.git.ungureanupaulsebastian@gmail.com>

> Subject: stash: implement the "list" command in the builtin

Nit: The previous commit messages all have the format "stash: convert
<command> to builtin", maybe follow the same pattern here?

The rest of the patch looks good to me.

On 08/08, Paul-Sebastian Ungureanu wrote:
> Add stash list to the helper and delete the list_stash function
> from the shell script.
> 
> Signed-off-by: Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com>
> ---
>  builtin/stash--helper.c | 31 +++++++++++++++++++++++++++++++
>  git-stash.sh            |  7 +------
>  2 files changed, 32 insertions(+), 6 deletions(-)
> 
> diff --git a/builtin/stash--helper.c b/builtin/stash--helper.c
> index d6bd468e0..daa4d0034 100644
> --- a/builtin/stash--helper.c
> +++ b/builtin/stash--helper.c
> @@ -12,6 +12,7 @@
>  #include "rerere.h"
>  
>  static const char * const git_stash_helper_usage[] = {
> +	N_("git stash--helper list [<options>]"),
>  	N_("git stash--helper drop [-q|--quiet] [<stash>]"),
>  	N_("git stash--helper ( pop | apply ) [--index] [-q|--quiet] [<stash>]"),
>  	N_("git stash--helper branch <branchname> [<stash>]"),
> @@ -19,6 +20,11 @@ static const char * const git_stash_helper_usage[] = {
>  	NULL
>  };
>  
> +static const char * const git_stash_helper_list_usage[] = {
> +	N_("git stash--helper list [<options>]"),
> +	NULL
> +};
> +
>  static const char * const git_stash_helper_drop_usage[] = {
>  	N_("git stash--helper drop [-q|--quiet] [<stash>]"),
>  	NULL
> @@ -609,6 +615,29 @@ static int branch_stash(int argc, const char **argv, const char *prefix)
>  	return ret;
>  }
>  
> +static int list_stash(int argc, const char **argv, const char *prefix)
> +{
> +	struct child_process cp = CHILD_PROCESS_INIT;
> +	struct option options[] = {
> +		OPT_END()
> +	};
> +
> +	argc = parse_options(argc, argv, prefix, options,
> +			     git_stash_helper_list_usage,
> +			     PARSE_OPT_KEEP_UNKNOWN);
> +
> +	if (!ref_exists(ref_stash))
> +		return 0;
> +
> +	cp.git_cmd = 1;
> +	argv_array_pushl(&cp.args, "log", "--format=%gd: %gs", "-g",
> +			 "--first-parent", "-m", NULL);
> +	argv_array_pushv(&cp.args, argv);
> +	argv_array_push(&cp.args, ref_stash);
> +	argv_array_push(&cp.args, "--");
> +	return run_command(&cp);
> +}
> +
>  int cmd_stash__helper(int argc, const char **argv, const char *prefix)
>  {
>  	pid_t pid = getpid();
> @@ -639,6 +668,8 @@ int cmd_stash__helper(int argc, const char **argv, const char *prefix)
>  		return !!pop_stash(argc, argv, prefix);
>  	else if (!strcmp(argv[0], "branch"))
>  		return !!branch_stash(argc, argv, prefix);
> +	else if (!strcmp(argv[0], "list"))
> +		return !!list_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 8f2640fe9..6052441aa 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -382,11 +382,6 @@ have_stash () {
>  	git rev-parse --verify --quiet $ref_stash >/dev/null
>  }
>  
> -list_stash () {
> -	have_stash || return 0
> -	git log --format="%gd: %gs" -g --first-parent -m "$@" $ref_stash --
> -}
> -
>  show_stash () {
>  	ALLOW_UNKNOWN_FLAGS=t
>  	assert_stash_like "$@"
> @@ -574,7 +569,7 @@ test -n "$seen_non_option" || set "push" "$@"
>  case "$1" in
>  list)
>  	shift
> -	list_stash "$@"
> +	git stash--helper list "$@"
>  	;;
>  show)
>  	shift
> -- 
> 2.18.0.573.g56500d98f
>