On Sat, Mar 24, 2018 at 2:23 PM, Paul-Sebastian Ungureanu
<[email protected]> wrote:
> Currently, because git stash is not fully converted to C, I
> introduced a new helper that will hold the converted commands.
> ---
> diff --git a/builtin/stash--helper.c b/builtin/stash--helper.c
> @@ -0,0 +1,52 @@
> +int cmd_stash__helper(int argc, const char **argv, const char *prefix)
> +{
> + int cmdmode = 0;
> +
> + struct option options[] = {
> + OPT_CMDMODE(0, "list", &cmdmode,
> + N_("list stash entries"), LIST_STASH),
> + OPT_END()
> + };
Is the intention that once git-stash--helper implements all 'stash'
functionality, you will simply rename git-stash--helper to git-stash?
If so, then I'd think that you'd want it to accept subcommand
arguments as bare words ("apply", "drop") in order to be consistent
with the existing git-stash command set, not in dashed form
("--apply", "--drop"). In that case, OPT_CMDMODE doesn't seem
appropriate. Instead, you should be consulting argv[] directly (as in
[1]) after parse_options().
[1]: https://public-inbox.org/git/[email protected]/
> + argc = parse_options(argc, argv, prefix, options,
> + git_stash__helper_usage, PARSE_OPT_KEEP_UNKNOWN);
> +
> + if (!cmdmode)
> + usage_with_options(git_stash__helper_usage, options);
> +
> + switch (cmdmode) {
> + case LIST_STASH:
> + return list_stash(argc, argv, prefix);
> + }
> + return 0;
> +}
> diff --git a/git.c b/git.c
> @@ -466,6 +466,7 @@ static struct cmd_struct commands[] = {
> { "show-branch", cmd_show_branch, RUN_SETUP },
> { "show-ref", cmd_show_ref, RUN_SETUP },
> { "stage", cmd_add, RUN_SETUP | NEED_WORK_TREE },
> + { "stash--helper", cmd_stash__helper, RUN_SETUP },
You don't require a working tree? Seems odd for git-stash.
> { "status", cmd_status, RUN_SETUP | NEED_WORK_TREE },
> { "stripspace", cmd_stripspace },
> { "submodule--helper", cmd_submodule__helper, RUN_SETUP |
> SUPPORT_SUPER_PREFIX},
