Thread (167 messages) 167 messages, 8 authors, 2018-11-09

Re: [GSoC][PATCH v7 23/26] stash: convert `stash--helper.c` into `stash.c`

From: Thomas Gummerer <hidden>
Date: 2018-08-18 16:51:07

On 08/08, Paul-Sebastian Ungureanu wrote:
quoted hunk ↗ jump to hunk
The old shell script `git-stash.sh`  was removed and replaced
entirely by `builtin/stash.c`. In order to do that, `create` and
`push` were adapted to work without `stash.sh`. For example, before
this commit, `git stash create` called `git stash--helper create
--message "$*"`. If it called `git stash--helper create "$@"`, then
some of these changes wouldn't have been necessary.

This commit also removes the word `helper` since now stash is
called directly and not by a shell script.
---
 .gitignore                           |   1 -
 Makefile                             |   3 +-
 builtin.h                            |   2 +-
 builtin/{stash--helper.c => stash.c} | 132 ++++++++++++-----------
 git-stash.sh                         | 153 ---------------------------
 git.c                                |   2 +-
 6 files changed, 74 insertions(+), 219 deletions(-)
 rename builtin/{stash--helper.c => stash.c} (91%)
 delete mode 100755 git-stash.sh
diff --git a/builtin/stash--helper.c b/builtin/stash.c
similarity index 91%
rename from builtin/stash--helper.c
rename to builtin/stash.c
index f54a476e3..0ef88408a 100644
--- a/builtin/stash--helper.c
+++ b/builtin/stash.c
[...]
@@ -1445,9 +1448,10 @@ static int push_stash(int argc, const char **argv, const char *prefix)
 		OPT_END()
 	};
 
-	argc = parse_options(argc, argv, prefix, options,
-			     git_stash_helper_push_usage,
-			     0);
+	if (argc)
+		argc = parse_options(argc, argv, prefix, options,
+				     git_stash_push_usage,
+				     0);
This change is a bit surprising here.  Why is this necessary?  I
thought parse_options would handle no arguments just fine?
quoted hunk ↗ jump to hunk
 	return do_push_stash(argc, argv, prefix, keep_index, patch_mode,
 			     include_untracked, quiet, stash_msg);
@@ -1479,7 +1483,7 @@ static int save_stash(int argc, const char **argv, const char *prefix)
 	};
 
 	argc = parse_options(argc, argv, prefix, options,
-			     git_stash_helper_save_usage,
+			     git_stash_save_usage,
 			     0);
 
 	for (i = 0; i < argc; ++i)
@@ -1491,7 +1495,7 @@ static int save_stash(int argc, const char **argv, const char *prefix)
 			     include_untracked, quiet, stash_msg);
 }
 
-int cmd_stash__helper(int argc, const char **argv, const char *prefix)
+int cmd_stash(int argc, const char **argv, const char *prefix)
 {
 	pid_t pid = getpid();
 	const char *index_file;
@@ -1502,16 +1506,16 @@ int cmd_stash__helper(int argc, const char **argv, const char *prefix)
 
 	git_config(git_default_config, NULL);
 
-	argc = parse_options(argc, argv, prefix, options, git_stash_helper_usage,
+	argc = parse_options(argc, argv, prefix, options, git_stash_usage,
 			     PARSE_OPT_KEEP_UNKNOWN | PARSE_OPT_KEEP_DASHDASH);
 
 	index_file = get_index_file();
 	strbuf_addf(&stash_index_path, "%s.stash.%" PRIuMAX, index_file,
 		    (uintmax_t)pid);
 
-	if (argc < 1)
-		usage_with_options(git_stash_helper_usage, options);
-	if (!strcmp(argv[0], "apply"))
+	if (argc == 0)
+		return !!push_stash(0, NULL, prefix);
+	else if (!strcmp(argv[0], "apply"))
 		return !!apply_stash(argc, argv, prefix);
 	else if (!strcmp(argv[0], "clear"))
 		return !!clear_stash(argc, argv, prefix);
@@ -1533,7 +1537,13 @@ int cmd_stash__helper(int argc, const char **argv, const char *prefix)
 		return !!push_stash(argc, argv, prefix);
 	else if (!strcmp(argv[0], "save"))
 		return !!save_stash(argc, argv, prefix);
+	if (*argv[0] == '-') {
+		struct argv_array args = ARGV_ARRAY_INIT;
+		argv_array_push(&args, "push");
+		argv_array_pushv(&args, argv);
+		return !!push_stash(args.argc, args.argv, prefix);
+	}
This is a bit different than what the current code does.  Currently
the rules for when a plain 'git stash' becomes 'git stash push' are
the following:

- If there are no arguments.
- If all arguments are option arguments.
- If the first argument of 'git stash' is '-p'.
- If the first argument of 'git stash' is '--'.

This is to avoid someone typing 'git stash -q drop' for example, and
then being surprised that a new stash was created instead of an old
one being dropped, which what we have above would do.

For more reasoning about these aliasing rules see also the thread at [1].

[1]: https://public-inbox.org/git/20170213200950.m3bcyp52wd25p737@sigill.intra.peff.net/
quoted hunk ↗ jump to hunk
 
 	usage_msg_opt(xstrfmt(_("unknown subcommand: %s"), argv[0]),
-		      git_stash_helper_usage, options);
+		      git_stash_usage, options);
 }
diff --git a/git-stash.sh b/git-stash.sh
deleted file mode 100755
index 695f1feba..000000000
--- a/git-stash.sh
+++ /dev/null
@@ -1,153 +0,0 @@
[...]
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help