On 08/08, Paul-Sebastian Ungureanu wrote:
> 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?

>       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.m3bcyp52wd25p...@sigill.intra.peff.net/

>  
>       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 @@
>
> [...]

Reply via email to