Re: [RFC][PATCH] git-stash: convert git stash list to C builtin
From: Eric Sunshine <hidden>
Date: 2018-03-25 07:08:49
On Sat, Mar 24, 2018 at 2:23 PM, Paul-Sebastian Ungureanu [off-list ref] wrote:
quoted hunk ↗ jump to hunk
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/20180324173707.17699-2-joel@teichroeb.net/
quoted hunk ↗ jump to hunk
+ 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},