[PATCH v4 3/5] stash: convert drop and clear to builtin

2018-03-28 Thread Joel Teichroeb
Add the drop and clear commands to the builtin helper. These two
are each simple, but are being added together as they are quite
related.

We have to unfortunately keep the drop and clear functions in the
shell script as functions are called with parameters internally
that are not valid when the commands are called externally. Once
pop is converted they can both be removed.

Signed-off-by: Joel Teichroeb 
---
 builtin/stash--helper.c | 101 
 git-stash.sh|   4 +-
 2 files changed, 103 insertions(+), 2 deletions(-)

diff --git a/builtin/stash--helper.c b/builtin/stash--helper.c
index 00e854734..640c545f5 100644
--- a/builtin/stash--helper.c
+++ b/builtin/stash--helper.c
@@ -11,7 +11,14 @@
 #include "dir.h"
 
 static const char * const git_stash_helper_usage[] = {
+   N_("git stash--helper drop [-q|--quiet] []"),
N_("git stash--helper apply [--index] [-q|--quiet] []"),
+   N_("git stash--helper clear"),
+   NULL
+};
+
+static const char * const git_stash_helper_drop_usage[] = {
+   N_("git stash--helper drop [-q|--quiet] []"),
NULL
 };
 
@@ -20,6 +27,11 @@ static const char * const git_stash_helper_apply_usage[] = {
NULL
 };
 
+static const char * const git_stash_helper_clear_usage[] = {
+   N_("git stash--helper clear"),
+   NULL
+};
+
 static const char *ref_stash = "refs/stash";
 static int quiet;
 static char stash_index_path[PATH_MAX];
@@ -168,6 +180,29 @@ static int get_stash_info(struct stash_info *info, int 
argc, const char **argv)
return 0;
 }
 
+static int do_clear_stash(void)
+{
+   struct object_id obj;
+   if (get_oid(ref_stash, &obj))
+   return 0;
+
+   return delete_ref(NULL, ref_stash, &obj, 0);
+}
+
+static int clear_stash(int argc, const char **argv, const char *prefix)
+{
+   struct option options[] = {
+   OPT_END()
+   };
+
+   argc = parse_options(argc, argv, prefix, options, 
git_stash_helper_clear_usage, PARSE_OPT_STOP_AT_NON_OPTION);
+
+   if (argc != 0)
+   return error(_("git stash--helper clear with parameters is 
unimplemented"));
+
+   return do_clear_stash();
+}
+
 static int reset_tree(struct object_id *i_tree, int update, int reset)
 {
struct unpack_trees_options opts;
@@ -412,6 +447,68 @@ static int apply_stash(int argc, const char **argv, const 
char *prefix)
return ret;
 }
 
+static int do_drop_stash(const char *prefix, struct stash_info *info)
+{
+   struct argv_array args = ARGV_ARRAY_INIT;
+   int ret;
+   struct child_process cp = CHILD_PROCESS_INIT;
+
+   argv_array_pushl(&args, "reflog", "delete", "--updateref", "--rewrite", 
NULL);
+   argv_array_push(&args, info->revision.buf);
+   ret = cmd_reflog(args.argc, args.argv, prefix);
+   if (!ret) {
+   if (!quiet)
+   printf(_("Dropped %s (%s)\n"), info->revision.buf, 
oid_to_hex(&info->w_commit));
+   } else {
+   return error(_("%s: Could not drop stash entry"), 
info->revision.buf);
+   }
+
+   cp.git_cmd = 1;
+   /* Even though --quiet is specified, rev-parse still outputs the hash */
+   cp.no_stdout = 1;
+   argv_array_pushl(&cp.args, "rev-parse", "--verify", "--quiet", NULL);
+   argv_array_pushf(&cp.args, "%s@{0}", ref_stash);
+   ret = run_command(&cp);
+
+   if (ret)
+   do_clear_stash();
+
+   return 0;
+}
+
+static int assert_stash_ref(struct stash_info *info)
+{
+   if (!info->is_stash_ref)
+   return error(_("'%s' is not a stash reference"), 
info->revision.buf);
+
+   return 0;
+}
+
+static int drop_stash(int argc, const char **argv, const char *prefix)
+{
+   struct stash_info info;
+   int ret;
+   struct option options[] = {
+   OPT__QUIET(&quiet, N_("be quiet, only report errors")),
+   OPT_END()
+   };
+
+   argc = parse_options(argc, argv, prefix, options,
+   git_stash_helper_drop_usage, 0);
+
+   if (get_stash_info(&info, argc, argv))
+   return -1;
+
+   if (assert_stash_ref(&info)) {
+   free_stash_info(&info);
+   return -1;
+   }
+
+   ret = do_drop_stash(prefix, &info);
+   free_stash_info(&info);
+   return ret;
+}
+
 int cmd_stash__helper(int argc, const char **argv, const char *prefix)
 {
int result = 0;
@@ -434,6 +531,10 @@ int cmd_stash__helper(int argc, const char **argv, const 
char *prefix)
usage_with_options(git_stash_helper_usage, options);
else if (!strcmp(argv[0], "apply"))
result = apply_stash(argc, argv, prefix);
+   else if (!strcmp(argv[0], "clear"))
+   result = clear_stash(argc, argv, prefix);
+   else if (!strcmp(argv[0], "drop"))
+   result = drop_stash(argc, argv, prefix);
else {
error(_("unknown subcommand: 

Re: [PATCH v4 3/5] stash: convert drop and clear to builtin

2018-03-30 Thread Junio C Hamano
Joel Teichroeb  writes:

> +static int do_clear_stash(void)
> +{
> + struct object_id obj;
> + if (get_oid(ref_stash, &obj))
> + return 0;
> + return delete_ref(NULL, ref_stash, &obj, 0);
> +}

Here you see if the "refs/stash" is there, and learn what its
current value is, using get_oid(), so that you can call delete_ref()
with it.

> +static int do_drop_stash(const char *prefix, struct stash_info *info)
> +{
> + ...
> + cp.git_cmd = 1;
> + /* Even though --quiet is specified, rev-parse still outputs the hash */
> + cp.no_stdout = 1;
> + argv_array_pushl(&cp.args, "rev-parse", "--verify", "--quiet", NULL);
> + argv_array_pushf(&cp.args, "%s@{0}", ref_stash);
> + ret = run_command(&cp);
> + if (ret)
> + do_clear_stash();

Here you call out to rev-parse as an external process.  Isn't doing
the same get_oid() sufficient?

Not limited to the above examples, the conversion in this series
feels somewhat unbalanced---doing easy things like this by forking
an external process and then doing a relatively heavyweight thing
like merge operation (in 2/5) in-process feels the other way around.

Thanks.