Re: [GSoC][PATCH v7 09/26] stash: implement the "list" command in the builtin

[Paul Sebastian Ungureanu]

On Wed, Aug 15, 2018 at 10:41 PM, Thomas Gummerer  wrote:
>> Subject: stash: implement the "list" command in the builtin
>
> Nit: The previous commit messages all have the format "stash: convert
>  to builtin", maybe follow the same pattern here?

At first, the subject of the commit matched the pattern. I think I
changed it in order
to avoid any confusion with "list" as data structure. Now, I guess
that "stash:" at the
beginning of the commit message removes any doubt.

Re: [GSoC][PATCH v7 09/26] stash: implement the "list" command in the builtin

[Thomas Gummerer]

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

Nit: The previous commit messages all have the format "stash: convert
 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 
> ---
>  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 []"),
>   N_("git stash--helper drop [-q|--quiet] []"),
>   N_("git stash--helper ( pop | apply ) [--index] [-q|--quiet] 
> []"),
>   N_("git stash--helper branch  []"),
> @@ -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 []"),
> + NULL
> +};
> +
>  static const char * const git_stash_helper_drop_usage[] = {
>   N_("git stash--helper drop [-q|--quiet] []"),
>   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(, "log", "--format=%gd: %gs", "-g",
> +  "--first-parent", "-m", NULL);
> + argv_array_pushv(, argv);
> + argv_array_push(, ref_stash);
> + argv_array_push(, "--");
> + return run_command();
> +}
> +
>  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
> 

[GSoC][PATCH v7 09/26] stash: implement the "list" command in the builtin

[Paul-Sebastian Ungureanu]

Add stash list to the helper and delete the list_stash function
from the shell script.

Signed-off-by: Paul-Sebastian Ungureanu 
---
 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 []"),
N_("git stash--helper drop [-q|--quiet] []"),
N_("git stash--helper ( pop | apply ) [--index] [-q|--quiet] 
[]"),
N_("git stash--helper branch  []"),
@@ -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 []"),
+   NULL
+};
+
 static const char * const git_stash_helper_drop_usage[] = {
N_("git stash--helper drop [-q|--quiet] []"),
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(, "log", "--format=%gd: %gs", "-g",
+"--first-parent", "-m", NULL);
+   argv_array_pushv(, argv);
+   argv_array_push(, ref_stash);
+   argv_array_push(, "--");
+   return run_command();
+}
+
 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