diff mbox series

[v11,20/22] stash: convert `stash--helper.c` into `stash.c`

Message ID de2ebf96ee5f94a1cdadee67e436f5bd1b0fb6a5.1542925164.git.ungureanupaulsebastian@gmail.com (mailing list archive)
State New, archived
Headers show
Series Convert "git stash" to C builtin | expand

Commit Message

Paul-Sebastian Ungureanu Nov. 22, 2018, 11:05 p.m. UTC
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.

Signed-off-by: Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com>
---
 .gitignore                           |   1 -
 Makefile                             |   3 +-
 builtin.h                            |   2 +-
 builtin/{stash--helper.c => stash.c} | 157 +++++++++++++++------------
 git-stash.sh                         | 153 --------------------------
 git.c                                |   2 +-
 6 files changed, 92 insertions(+), 226 deletions(-)
 rename builtin/{stash--helper.c => stash.c} (91%)
 delete mode 100755 git-stash.sh

Comments

Junio C Hamano Nov. 26, 2018, 5:30 a.m. UTC | #1
Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com> writes:

> 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.
>
> Signed-off-by: Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com>
> ---
>  .gitignore                           |   1 -
>  Makefile                             |   3 +-
>  builtin.h                            |   2 +-
>  builtin/{stash--helper.c => stash.c} | 157 +++++++++++++++------------
>  git-stash.sh                         | 153 --------------------------
>  git.c                                |   2 +-
>  6 files changed, 92 insertions(+), 226 deletions(-)
>  rename builtin/{stash--helper.c => stash.c} (91%)
>  delete mode 100755 git-stash.sh

Seeing the recent trouble in "rebase in C" and how keeping the
scripted version as "git legacy-rebase" helped us postpone the
rewritten version without ripping the whole thing out, I wonder if
we can do the same here.

Also, the remaining two patches should be done _before_ this step, I
would think.  I can understand it if the reason you have those two
after this step is because you found the opportunity for these
improvements after you wrote this step, but in the larger picture
seen by the end users of the "stash in C" and those developers who
follow the evolution of the code, the logical place for this "now we
have everything in C, we retire the scripted version" step to happen
is at the very end.

Thanks.
Johannes Schindelin Nov. 27, 2018, 1:46 p.m. UTC | #2
Hi Junio & Paul,

On Mon, 26 Nov 2018, Junio C Hamano wrote:

> Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com> writes:
> 
> > 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.
> >
> > Signed-off-by: Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com>
> > ---
> >  .gitignore                           |   1 -
> >  Makefile                             |   3 +-
> >  builtin.h                            |   2 +-
> >  builtin/{stash--helper.c => stash.c} | 157 +++++++++++++++------------
> >  git-stash.sh                         | 153 --------------------------
> >  git.c                                |   2 +-
> >  6 files changed, 92 insertions(+), 226 deletions(-)
> >  rename builtin/{stash--helper.c => stash.c} (91%)
> >  delete mode 100755 git-stash.sh
> 
> Seeing the recent trouble in "rebase in C" and how keeping the
> scripted version as "git legacy-rebase" helped us postpone the
> rewritten version without ripping the whole thing out, I wonder if
> we can do the same here.

Feel very free to cherry-pick
https://github.com/git-for-windows/git/commit/004da7e7faa36c872868ae938e06594ea1c2f01c
and
https://github.com/git-for-windows/git/commit/cedfcd39f5a4e4beb33e16fa67c4659fd4bdabf6
which is what we carry in Git for Windows.

Ciao,
Dscho

> Also, the remaining two patches should be done _before_ this step, I
> would think.  I can understand it if the reason you have those two
> after this step is because you found the opportunity for these
> improvements after you wrote this step, but in the larger picture
> seen by the end users of the "stash in C" and those developers who
> follow the evolution of the code, the logical place for this "now we
> have everything in C, we retire the scripted version" step to happen
> is at the very end.
> 
> Thanks.
>
Ævar Arnfjörð Bjarmason Nov. 27, 2018, 11:40 p.m. UTC | #3
On Tue, Nov 27 2018, Johannes Schindelin wrote:

> Hi Junio & Paul,
>
> On Mon, 26 Nov 2018, Junio C Hamano wrote:
>
>> Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com> writes:
>>
>> > 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.
>> >
>> > Signed-off-by: Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com>
>> > ---
>> >  .gitignore                           |   1 -
>> >  Makefile                             |   3 +-
>> >  builtin.h                            |   2 +-
>> >  builtin/{stash--helper.c => stash.c} | 157 +++++++++++++++------------
>> >  git-stash.sh                         | 153 --------------------------
>> >  git.c                                |   2 +-
>> >  6 files changed, 92 insertions(+), 226 deletions(-)
>> >  rename builtin/{stash--helper.c => stash.c} (91%)
>> >  delete mode 100755 git-stash.sh
>>
>> Seeing the recent trouble in "rebase in C" and how keeping the
>> scripted version as "git legacy-rebase" helped us postpone the
>> rewritten version without ripping the whole thing out, I wonder if
>> we can do the same here.
>
> Feel very free to cherry-pick
> https://github.com/git-for-windows/git/commit/004da7e7faa36c872868ae938e06594ea1c2f01c
> and
> https://github.com/git-for-windows/git/commit/cedfcd39f5a4e4beb33e16fa67c4659fd4bdabf6
> which is what we carry in Git for Windows.

...and then something similar to 62c23938fa ("tests: add a special setup
where rebase.useBuiltin is off", 2018-11-14) so those of us who're
smoking next for bugs can test both and report if some of the test
setups (odd OS's etc) show a difference in behavior.

I did some of this the last time around, but then I had to e.g. smoke
next against pu, and look at the general fallout there and see what was
due to stash-in-C, it would be much better to have a
GIT_TEST_STASH_USE_BUILTIN.


>> Also, the remaining two patches should be done _before_ this step, I
>> would think.  I can understand it if the reason you have those two
>> after this step is because you found the opportunity for these
>> improvements after you wrote this step, but in the larger picture
>> seen by the end users of the "stash in C" and those developers who
>> follow the evolution of the code, the logical place for this "now we
>> have everything in C, we retire the scripted version" step to happen
>> is at the very end.
>>
>> Thanks.
>>
Johannes Schindelin Nov. 29, 2018, 10:58 a.m. UTC | #4
Hi,

On Wed, 28 Nov 2018, Ævar Arnfjörð Bjarmason wrote:

> On Tue, Nov 27 2018, Johannes Schindelin wrote:
> 
> > On Mon, 26 Nov 2018, Junio C Hamano wrote:
> >
> >> Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com> writes:
> >>
> >> > 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.
> >>
> >> Seeing the recent trouble in "rebase in C" and how keeping the
> >> scripted version as "git legacy-rebase" helped us postpone the
> >> rewritten version without ripping the whole thing out, I wonder if
> >> we can do the same here.
> >
> > Feel very free to cherry-pick
> > https://github.com/git-for-windows/git/commit/004da7e7faa36c872868ae938e06594ea1c2f01c
> > and
> > https://github.com/git-for-windows/git/commit/cedfcd39f5a4e4beb33e16fa67c4659fd4bdabf6
> > which is what we carry in Git for Windows.
> 
> ...and then something similar to 62c23938fa ("tests: add a special setup
> where rebase.useBuiltin is off", 2018-11-14) so those of us who're
> smoking next for bugs can test both and report if some of the test
> setups (odd OS's etc) show a difference in behavior.

I allowed myself to make those changes, and to reorder the last three
patches as asked by Junio. Paul, please find the result at
https://github.com/dscho/git as `git-stash`. If you agree with it, I would
be delighted if you resubmitted it directly after Git v2.20.0 is released
(Junio, it seemed that -rc1 slipped a couple of days, and now -rc2, too,
any word when you think the final v2.20.0 is due?).

Thanks,
Dscho
diff mbox series

Patch

diff --git a/.gitignore b/.gitignore
index 6ecab90ab2..0d77ea5894 100644
--- a/.gitignore
+++ b/.gitignore
@@ -162,7 +162,6 @@ 
 /git-show-ref
 /git-stage
 /git-stash
-/git-stash--helper
 /git-status
 /git-stripspace
 /git-submodule
diff --git a/Makefile b/Makefile
index aa83545e94..450936fcaf 100644
--- a/Makefile
+++ b/Makefile
@@ -619,7 +619,6 @@  SCRIPT_SH += git-quiltimport.sh
 SCRIPT_SH += git-legacy-rebase.sh
 SCRIPT_SH += git-remote-testgit.sh
 SCRIPT_SH += git-request-pull.sh
-SCRIPT_SH += git-stash.sh
 SCRIPT_SH += git-submodule.sh
 SCRIPT_SH += git-web--browse.sh
 
@@ -1115,7 +1114,7 @@  BUILTIN_OBJS += builtin/shortlog.o
 BUILTIN_OBJS += builtin/show-branch.o
 BUILTIN_OBJS += builtin/show-index.o
 BUILTIN_OBJS += builtin/show-ref.o
-BUILTIN_OBJS += builtin/stash--helper.o
+BUILTIN_OBJS += builtin/stash.o
 BUILTIN_OBJS += builtin/stripspace.o
 BUILTIN_OBJS += builtin/submodule--helper.o
 BUILTIN_OBJS += builtin/symbolic-ref.o
diff --git a/builtin.h b/builtin.h
index ff4460aff7..b78ab6e30b 100644
--- a/builtin.h
+++ b/builtin.h
@@ -225,7 +225,7 @@  extern int cmd_show(int argc, const char **argv, const char *prefix);
 extern int cmd_show_branch(int argc, const char **argv, const char *prefix);
 extern int cmd_show_index(int argc, const char **argv, const char *prefix);
 extern int cmd_status(int argc, const char **argv, const char *prefix);
-extern int cmd_stash__helper(int argc, const char **argv, const char *prefix);
+extern int cmd_stash(int argc, const char **argv, const char *prefix);
 extern int cmd_stripspace(int argc, const char **argv, const char *prefix);
 extern int cmd_submodule__helper(int argc, const char **argv, const char *prefix);
 extern int cmd_symbolic_ref(int argc, const char **argv, const char *prefix);
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 47a0ab6669..c76a1936d5 100644
--- a/builtin/stash--helper.c
+++ b/builtin/stash.c
@@ -16,75 +16,70 @@ 
 
 #define INCLUDE_ALL_FILES 2
 
-static const char * const git_stash_helper_usage[] = {
-	N_("git stash--helper list [<options>]"),
-	N_("git stash--helper show [<options>] [<stash>]"),
-	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>]"),
-	N_("git stash--helper clear"),
-	N_("git stash--helper [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n"
+static const char * const git_stash_usage[] = {
+	N_("git stash list [<options>]"),
+	N_("git stash show [<options>] [<stash>]"),
+	N_("git stash drop [-q|--quiet] [<stash>]"),
+	N_("git stash ( pop | apply ) [--index] [-q|--quiet] [<stash>]"),
+	N_("git stash branch <branchname> [<stash>]"),
+	N_("git stash clear"),
+	N_("git stash [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n"
 	   "          [-u|--include-untracked] [-a|--all] [-m|--message <message>]\n"
 	   "          [--] [<pathspec>...]]"),
-	N_("git stash--helper save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n"
+	N_("git stash save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n"
 	   "          [-u|--include-untracked] [-a|--all] [<message>]"),
 	NULL
 };
 
-static const char * const git_stash_helper_list_usage[] = {
-	N_("git stash--helper list [<options>]"),
+static const char * const git_stash_list_usage[] = {
+	N_("git stash list [<options>]"),
 	NULL
 };
 
-static const char * const git_stash_helper_show_usage[] = {
-	N_("git stash--helper show [<options>] [<stash>]"),
+static const char * const git_stash_show_usage[] = {
+	N_("git stash show [<options>] [<stash>]"),
 	NULL
 };
 
-static const char * const git_stash_helper_drop_usage[] = {
-	N_("git stash--helper drop [-q|--quiet] [<stash>]"),
+static const char * const git_stash_drop_usage[] = {
+	N_("git stash drop [-q|--quiet] [<stash>]"),
 	NULL
 };
 
-static const char * const git_stash_helper_pop_usage[] = {
-	N_("git stash--helper pop [--index] [-q|--quiet] [<stash>]"),
+static const char * const git_stash_pop_usage[] = {
+	N_("git stash pop [--index] [-q|--quiet] [<stash>]"),
 	NULL
 };
 
-static const char * const git_stash_helper_apply_usage[] = {
-	N_("git stash--helper apply [--index] [-q|--quiet] [<stash>]"),
+static const char * const git_stash_apply_usage[] = {
+	N_("git stash apply [--index] [-q|--quiet] [<stash>]"),
 	NULL
 };
 
-static const char * const git_stash_helper_branch_usage[] = {
-	N_("git stash--helper branch <branchname> [<stash>]"),
+static const char * const git_stash_branch_usage[] = {
+	N_("git stash branch <branchname> [<stash>]"),
 	NULL
 };
 
-static const char * const git_stash_helper_clear_usage[] = {
-	N_("git stash--helper clear"),
+static const char * const git_stash_clear_usage[] = {
+	N_("git stash clear"),
 	NULL
 };
 
-static const char * const git_stash_helper_store_usage[] = {
-	N_("git stash--helper store [-m|--message <message>] [-q|--quiet] <commit>"),
+static const char * const git_stash_store_usage[] = {
+	N_("git stash store [-m|--message <message>] [-q|--quiet] <commit>"),
 	NULL
 };
 
-static const char * const git_stash_helper_create_usage[] = {
-	N_("git stash--helper create [<message>]"),
-	NULL
-};
-
-static const char * const git_stash_helper_push_usage[] = {
-	N_("git stash--helper [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n"
+static const char * const git_stash_push_usage[] = {
+	N_("git stash [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n"
 	   "          [-u|--include-untracked] [-a|--all] [-m|--message <message>]\n"
 	   "          [--] [<pathspec>...]]"),
 	NULL
 };
 
-static const char * const git_stash_helper_save_usage[] = {
-	N_("git stash--helper save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n"
+static const char * const git_stash_save_usage[] = {
+	N_("git stash save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n"
 	   "          [-u|--include-untracked] [-a|--all] [<message>]"),
 	NULL
 };
@@ -221,7 +216,7 @@  static int clear_stash(int argc, const char **argv, const char *prefix)
 	};
 
 	argc = parse_options(argc, argv, prefix, options,
-			     git_stash_helper_clear_usage,
+			     git_stash_clear_usage,
 			     PARSE_OPT_STOP_AT_NON_OPTION);
 
 	if (argc)
@@ -526,7 +521,7 @@  static int apply_stash(int argc, const char **argv, const char *prefix)
 	};
 
 	argc = parse_options(argc, argv, prefix, options,
-			     git_stash_helper_apply_usage, 0);
+			     git_stash_apply_usage, 0);
 
 	if (get_stash_info(&info, argc, argv))
 		return -1;
@@ -599,7 +594,7 @@  static int drop_stash(int argc, const char **argv, const char *prefix)
 	};
 
 	argc = parse_options(argc, argv, prefix, options,
-			     git_stash_helper_drop_usage, 0);
+			     git_stash_drop_usage, 0);
 
 	if (get_stash_info(&info, argc, argv))
 		return -1;
@@ -625,7 +620,7 @@  static int pop_stash(int argc, const char **argv, const char *prefix)
 	};
 
 	argc = parse_options(argc, argv, prefix, options,
-			     git_stash_helper_pop_usage, 0);
+			     git_stash_pop_usage, 0);
 
 	if (get_stash_info(&info, argc, argv))
 		return -1;
@@ -652,7 +647,7 @@  static int branch_stash(int argc, const char **argv, const char *prefix)
 	};
 
 	argc = parse_options(argc, argv, prefix, options,
-			     git_stash_helper_branch_usage, 0);
+			     git_stash_branch_usage, 0);
 
 	if (!argc) {
 		fprintf_ln(stderr, _("No branch name specified"));
@@ -687,7 +682,7 @@  static int list_stash(int argc, const char **argv, const char *prefix)
 	};
 
 	argc = parse_options(argc, argv, prefix, options,
-			     git_stash_helper_list_usage,
+			     git_stash_list_usage,
 			     PARSE_OPT_KEEP_UNKNOWN);
 
 	if (!ref_exists(ref_stash))
@@ -767,7 +762,7 @@  static int show_stash(int argc, const char **argv, const char *prefix)
 	argc = setup_revisions(argc, argv, &rev, NULL);
 	if (argc > 1) {
 		free_stash_info(&info);
-		usage_with_options(git_stash_helper_show_usage, options);
+		usage_with_options(git_stash_show_usage, options);
 	}
 
 	rev.diffopt.flags.recursive = 1;
@@ -813,7 +808,7 @@  static int store_stash(int argc, const char **argv, const char *prefix)
 	};
 
 	argc = parse_options(argc, argv, prefix, options,
-			     git_stash_helper_store_usage,
+			     git_stash_store_usage,
 			     PARSE_OPT_KEEP_UNKNOWN);
 
 	if (argc != 1) {
@@ -1231,28 +1226,16 @@  static int do_create_stash(struct pathspec ps, struct strbuf *stash_msg_buf,
 
 static int create_stash(int argc, const char **argv, const char *prefix)
 {
-	int include_untracked = 0;
 	int ret = 0;
-	const char *stash_msg = NULL;
 	struct strbuf stash_msg_buf = STRBUF_INIT;
 	struct stash_info info;
 	struct pathspec ps;
-	struct option options[] = {
-		OPT_BOOL('u', "include-untracked", &include_untracked,
-			 N_("include untracked files in stash")),
-		OPT_STRING('m', "message", &stash_msg, N_("message"),
-			 N_("stash message")),
-		OPT_END()
-	};
 
-	argc = parse_options(argc, argv, prefix, options,
-			     git_stash_helper_create_usage,
-			     0);
+	/* Starting with argv[1], since argv[0] is "create" */
+	strbuf_join_argv(&stash_msg_buf, argc - 1, ++argv, ' ');
 
 	memset(&ps, 0, sizeof(ps));
-	strbuf_addstr(&stash_msg_buf, stash_msg);
-	ret = do_create_stash(ps, &stash_msg_buf, include_untracked, 0, &info,
-			      NULL, 0);
+	ret = do_create_stash(ps, &stash_msg_buf, 0, 0, &info, NULL, 0);
 
 	if (!ret)
 		printf_ln("%s", oid_to_hex(&info.w_commit));
@@ -1487,9 +1470,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);
 
 	parse_pathspec(&ps, 0, PATHSPEC_PREFER_FULL, prefix, argv);
 	return do_push_stash(ps, stash_msg, quiet, keep_index, patch_mode,
@@ -1522,7 +1506,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,
 			     PARSE_OPT_KEEP_DASHDASH);
 
 	if (argc)
@@ -1536,10 +1520,12 @@  static int save_stash(int argc, const char **argv, const char *prefix)
 	return ret;
 }
 
-int cmd_stash__helper(int argc, const char **argv, const char *prefix)
+int cmd_stash(int argc, const char **argv, const char *prefix)
 {
+	int i = -1;
 	pid_t pid = getpid();
 	const char *index_file;
+	struct argv_array args = ARGV_ARRAY_INIT;
 
 	struct option options[] = {
 		OPT_END()
@@ -1547,16 +1533,16 @@  int cmd_stash__helper(int argc, const char **argv, const char *prefix)
 
 	git_config(git_diff_basic_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)
+		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);
@@ -1578,7 +1564,42 @@  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);
+	else if (*argv[0] != '-')
+		usage_msg_opt(xstrfmt(_("unknown subcommand: %s"), argv[0]),
+			      git_stash_usage, options);
+
+	if (strcmp(argv[0], "-p")) {
+		while (++i < argc && strcmp(argv[i], "--")) {
+			/*
+			 * `akpqu` is a string which contains all short options,
+			 * except `-m` which is verified separately.
+			 */
+			if ((strlen(argv[i]) == 2) && *argv[i] == '-' &&
+			    strchr("akpqu", argv[i][1]))
+				continue;
+
+			if (!strcmp(argv[i], "--all") ||
+			    !strcmp(argv[i], "--keep-index") ||
+			    !strcmp(argv[i], "--no-keep-index") ||
+			    !strcmp(argv[i], "--patch") ||
+			    !strcmp(argv[i], "--quiet") ||
+			    !strcmp(argv[i], "--include-untracked"))
+				continue;
+
+			/*
+			 * `-m` and `--message=` are verified separately because
+			 * they need to be immediately followed by a string
+			 * (i.e.`-m"foobar"` or `--message="foobar"`).
+			 */
+			if (starts_with(argv[i], "-m") ||
+			    starts_with(argv[i], "--message="))
+				continue;
+
+			usage_with_options(git_stash_usage, options);
+		}
+	}
 
-	usage_msg_opt(xstrfmt(_("unknown subcommand: %s"), argv[0]),
-		      git_stash_helper_usage, options);
+	argv_array_push(&args, "push");
+	argv_array_pushv(&args, argv);
+	return !!push_stash(args.argc, args.argv, prefix);
 }
diff --git a/git-stash.sh b/git-stash.sh
deleted file mode 100755
index 695f1feba3..0000000000
--- a/git-stash.sh
+++ /dev/null
@@ -1,153 +0,0 @@ 
-#!/bin/sh
-# Copyright (c) 2007, Nanako Shiraishi
-
-dashless=$(basename "$0" | sed -e 's/-/ /')
-USAGE="list [<options>]
-   or: $dashless show [<stash>]
-   or: $dashless drop [-q|--quiet] [<stash>]
-   or: $dashless ( pop | apply ) [--index] [-q|--quiet] [<stash>]
-   or: $dashless branch <branchname> [<stash>]
-   or: $dashless save [--patch] [-k|--[no-]keep-index] [-q|--quiet]
-		      [-u|--include-untracked] [-a|--all] [<message>]
-   or: $dashless [push [--patch] [-k|--[no-]keep-index] [-q|--quiet]
-		       [-u|--include-untracked] [-a|--all] [-m <message>]
-		       [-- <pathspec>...]]
-   or: $dashless clear"
-
-SUBDIRECTORY_OK=Yes
-OPTIONS_SPEC=
-START_DIR=$(pwd)
-. git-sh-setup
-require_work_tree
-prefix=$(git rev-parse --show-prefix) || exit 1
-cd_to_toplevel
-
-TMP="$GIT_DIR/.git-stash.$$"
-TMPindex=${GIT_INDEX_FILE-"$(git rev-parse --git-path index)"}.stash.$$
-trap 'rm -f "$TMP-"* "$TMPindex"' 0
-
-ref_stash=refs/stash
-
-if git config --get-colorbool color.interactive; then
-       help_color="$(git config --get-color color.interactive.help 'red bold')"
-       reset_color="$(git config --get-color '' reset)"
-else
-       help_color=
-       reset_color=
-fi
-
-#
-# Parses the remaining options looking for flags and
-# at most one revision defaulting to ${ref_stash}@{0}
-# if none found.
-#
-# Derives related tree and commit objects from the
-# revision, if one is found.
-#
-# stash records the work tree, and is a merge between the
-# base commit (first parent) and the index tree (second parent).
-#
-#   REV is set to the symbolic version of the specified stash-like commit
-#   IS_STASH_LIKE is non-blank if ${REV} looks like a stash
-#   IS_STASH_REF is non-blank if the ${REV} looks like a stash ref
-#   s is set to the SHA1 of the stash commit
-#   w_commit is set to the commit containing the working tree
-#   b_commit is set to the base commit
-#   i_commit is set to the commit containing the index tree
-#   u_commit is set to the commit containing the untracked files tree
-#   w_tree is set to the working tree
-#   b_tree is set to the base tree
-#   i_tree is set to the index tree
-#   u_tree is set to the untracked files tree
-#
-#   GIT_QUIET is set to t if -q is specified
-#   INDEX_OPTION is set to --index if --index is specified.
-#   FLAGS is set to the remaining flags (if allowed)
-#
-# dies if:
-#   * too many revisions specified
-#   * no revision is specified and there is no stash stack
-#   * a revision is specified which cannot be resolve to a SHA1
-#   * a non-existent stash reference is specified
-#   * unknown flags were set and ALLOW_UNKNOWN_FLAGS is not "t"
-#
-
-test "$1" = "-p" && set "push" "$@"
-
-PARSE_CACHE='--not-parsed'
-# The default command is "push" if nothing but options are given
-seen_non_option=
-for opt
-do
-	case "$opt" in
-	--) break ;;
-	-*) ;;
-	*) seen_non_option=t; break ;;
-	esac
-done
-
-test -n "$seen_non_option" || set "push" "$@"
-
-# Main command set
-case "$1" in
-list)
-	shift
-	git stash--helper list "$@"
-	;;
-show)
-	shift
-	git stash--helper show "$@"
-	;;
-save)
-	shift
-	cd "$START_DIR"
-	git stash--helper save "$@"
-	;;
-push)
-	shift
-	cd "$START_DIR"
-	git stash--helper push "$@"
-	;;
-apply)
-	shift
-	cd "$START_DIR"
-	git stash--helper apply "$@"
-	;;
-clear)
-	shift
-	git stash--helper clear "$@"
-	;;
-create)
-	shift
-	git stash--helper create --message "$*"
-	;;
-store)
-	shift
-	git stash--helper store "$@"
-	;;
-drop)
-	shift
-	git stash--helper drop "$@"
-	;;
-pop)
-	shift
-	cd "$START_DIR"
-	git stash--helper pop "$@"
-	;;
-branch)
-	shift
-	cd "$START_DIR"
-	git stash--helper branch "$@"
-	;;
-*)
-	case $# in
-	0)
-		cd "$START_DIR"
-		git stash--helper push &&
-		say "$(gettext "(To restore them type \"git stash apply\")")"
-		;;
-	*)
-		usage
-	esac
-	;;
-esac
diff --git a/git.c b/git.c
index 76ee02802e..49ab91b4ec 100644
--- a/git.c
+++ b/git.c
@@ -554,7 +554,7 @@  static struct cmd_struct commands[] = {
 	{ "show-index", cmd_show_index },
 	{ "show-ref", cmd_show_ref, RUN_SETUP },
 	{ "stage", cmd_add, RUN_SETUP | NEED_WORK_TREE },
-	{ "stash--helper", cmd_stash__helper, RUN_SETUP | NEED_WORK_TREE },
+	{ "stash", cmd_stash, RUN_SETUP | NEED_WORK_TREE },
 	{ "status", cmd_status, RUN_SETUP | NEED_WORK_TREE },
 	{ "stripspace", cmd_stripspace },
 	{ "submodule--helper", cmd_submodule__helper, RUN_SETUP | SUPPORT_SUPER_PREFIX | NO_PARSEOPT },