{"thread":{"id":"30697","subject":"[RFC/PATCH] parse-options: introduce parse_subcommands","startedAt":"2012-06-04T09:35:34Z","lastAt":"2012-06-13T15:04:04Z","messageCount":13,"participants":["Ramkumar Ramachandra","Junio C Hamano","Jonathan Nieder","Felipe Contreras"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"192789","messageId":"1338802534-32394-1-git-send-email-artagnon@gmail.com","threadId":"30697","inReplyTo":null,"subject":"[RFC/PATCH] parse-options: introduce parse_subcommands","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2012-06-04T09:35:34Z","receivedAt":"2012-06-04T09:35:34Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Some git builtins like git-notes use subcommands to switch between\ndifferent modes of operation.  Introduce a parse_subcommands similar\nto parse_options to aid parsing these options.  The main advantage of\nusing it is that subcommands can have a descriptive help text attached\nto them.\n\nMake the git-notes builtin use parse_subcommands.  As a result, the\nfollowing error:\n\n  $ git notes foo\n  error: Unknown subcommand: foo\n  usage: git notes [--ref <notes_ref>] [list [<object>]]\n     or: git notes [--ref <notes_ref>] add [-f] [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]\n     or: git notes [--ref <notes_ref>] copy [-f] <from-object> <to-object>\n     or: git notes [--ref <notes_ref>] append [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]\n     or: git notes [--ref <notes_ref>] edit [<object>]\n     or: git notes [--ref <notes_ref>] show [<object>]\n     or: git notes [--ref <notes_ref>] merge [-v | -q] [-s <strategy> ] <notes_ref>\n     or: git notes merge --commit [-v | -q]\n     or: git notes merge --abort [-v | -q]\n     or: git notes [--ref <notes_ref>] remove [<object>...]\n     or: git notes [--ref <notes_ref>] prune [-n | -v]\n     or: git notes [--ref <notes_ref>] get-ref\n\n    --ref <notes_ref>     use notes from <notes_ref>\n\nis replaced by a more helpful:\n\n  $ git notes foo\n  error: unknown subcommand: foo\n  usage: git notes [<options>] [<subcommand>] [<options>] [<object>..]\n\n  available subcommands:\n      list                  list notes for given object\n      add                   add notes for given object\n      copy                  copy notes for first object onto second object\n      append                append notes to existing object\n      edit                  edit notes for given object\n      show                  show notes for given object\n      merge                 merge given notes ref into current notes ref\n      remove                remove notes for given objects\n      prune                 remove all notes for non-existing/unreachable objects\n      get-ref               print the current notes ref\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n I found this patch in one of my branches from a long time ago.  I\n haven't written documentation and tests yet because I'm not fully\n convinced that I like this.\n\n Thoughts?\n\n builtin/notes.c |   56 ++++++++++++++++++++++---------------------------------\n parse-options.c |   47 ++++++++++++++++++++++++++++++++++++++++++++++\n parse-options.h |   10 ++++++++++\n 3 files changed, 79 insertions(+), 34 deletions(-)\n\ndiff --git a/builtin/notes.c b/builtin/notes.c\nindex 3644d14..b1d6206 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -20,18 +20,7 @@\n #include \"notes-merge.h\"\n \n static const char * const git_notes_usage[] = {\n-\t\"git notes [--ref <notes_ref>] [list [<object>]]\",\n-\t\"git notes [--ref <notes_ref>] add [-f] [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]\",\n-\t\"git notes [--ref <notes_ref>] copy [-f] <from-object> <to-object>\",\n-\t\"git notes [--ref <notes_ref>] append [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]\",\n-\t\"git notes [--ref <notes_ref>] edit [<object>]\",\n-\t\"git notes [--ref <notes_ref>] show [<object>]\",\n-\t\"git notes [--ref <notes_ref>] merge [-v | -q] [-s <strategy> ] <notes_ref>\",\n-\t\"git notes merge --commit [-v | -q]\",\n-\t\"git notes merge --abort [-v | -q]\",\n-\t\"git notes [--ref <notes_ref>] remove [<object>...]\",\n-\t\"git notes [--ref <notes_ref>] prune [-n | -v]\",\n-\t\"git notes [--ref <notes_ref>] get-ref\",\n+\t\"git notes [<options>] [<subcommand>] [<options>] [<object>..]\",\n \tNULL\n };\n \n@@ -1081,28 +1070,27 @@ int cmd_notes(int argc, const char **argv, const char *prefix)\n \t\tstrbuf_release(&sb);\n \t}\n \n-\tif (argc < 1 || !strcmp(argv[0], \"list\"))\n-\t\tresult = list(argc, argv, prefix);\n-\telse if (!strcmp(argv[0], \"add\"))\n-\t\tresult = add(argc, argv, prefix);\n-\telse if (!strcmp(argv[0], \"copy\"))\n-\t\tresult = copy(argc, argv, prefix);\n-\telse if (!strcmp(argv[0], \"append\") || !strcmp(argv[0], \"edit\"))\n-\t\tresult = append_edit(argc, argv, prefix);\n-\telse if (!strcmp(argv[0], \"show\"))\n-\t\tresult = show(argc, argv, prefix);\n-\telse if (!strcmp(argv[0], \"merge\"))\n-\t\tresult = merge(argc, argv, prefix);\n-\telse if (!strcmp(argv[0], \"remove\"))\n-\t\tresult = remove_cmd(argc, argv, prefix);\n-\telse if (!strcmp(argv[0], \"prune\"))\n-\t\tresult = prune(argc, argv, prefix);\n-\telse if (!strcmp(argv[0], \"get-ref\"))\n-\t\tresult = get_ref(argc, argv, prefix);\n-\telse {\n-\t\tresult = error(_(\"Unknown subcommand: %s\"), argv[0]);\n-\t\tusage_with_options(git_notes_usage, options);\n-\t}\n+\tstruct subcommand subcmds[] = {\n+\t\t{ \"list\", \"list notes for given object\", list },\n+\t\t{ \"add\", \"add notes for given object\", add },\n+\t\t{ \"copy\",\n+\t\t  \"copy notes for first object onto second object\", copy },\n+\t\t{ \"append\", \"append notes to existing object\", append_edit },\n+\t\t{ \"edit\", \"edit notes for given object\", append_edit },\n+\t\t{ \"show\", \"show notes for given object\", show },\n+\t\t{ \"merge\",\n+\t\t  \"merge given notes ref into current notes ref\", merge },\n+\t\t{ \"remove\", \"remove notes for given objects\", remove_cmd },\n+\t\t{ \"prune\",\n+\t\t  \"remove all notes for non-existing/unreachable objects\", prune },\n+\t\t{ \"get-ref\", \"print the current notes ref\", get_ref },\n+\t\t{ NULL }\n+\t};\n \n+\tif (argc < 1)\n+\t\tresult = list(argc, argv, prefix);\n+\telse\n+\t\tresult = parse_subcommands(argc, argv, prefix,\n+\t\t\t\t\tsubcmds, git_notes_usage);\n \treturn result ? 1 : 0;\n }\ndiff --git a/parse-options.c b/parse-options.c\nindex f0098eb..76640fb 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -583,3 +583,50 @@ static int parse_options_usage(struct parse_opt_ctx_t *ctx,\n \treturn usage_with_options_internal(ctx, usagestr, opts, 0, err);\n }\n \n+static void subcommand_usage(const char * const *usagestr,\n+\t\t\tconst struct subcommand *subcmds)\n+{\n+\tconst struct subcommand *subcmd;\n+\n+\tfprintf(stderr, \"usage: %s\\n\", *usagestr++);\n+\twhile (*usagestr && **usagestr)\n+\t\tfprintf(stderr, \"   or: %s\\n\", *usagestr++);\n+\twhile (*usagestr) {\n+\t\tfprintf(stderr, \"%s%s\\n\",\n+\t\t\t\t**usagestr ? \"    \" : \"\",\n+\t\t\t\t*usagestr);\n+\t\tusagestr++;\n+\t}\n+\n+\tfputc('\\n', stderr);\n+\tfprintf(stderr, \"available subcommands:\\n\");\n+\tfor (subcmd = subcmds; subcmd->name != NULL; subcmd ++) {\n+\t\tsize_t pos;\n+\t\tint pad;\n+\n+\t\tpos = fprintf(stderr, \"    \");\n+\t\tpos += fprintf(stderr, \"%s\", subcmd->name);\n+\n+\t\tif (pos <= USAGE_OPTS_WIDTH)\n+\t\t\tpad = USAGE_OPTS_WIDTH - pos;\n+\t\telse {\n+\t\t\tfputc('\\n', stderr);\n+\t\t\tpad = USAGE_OPTS_WIDTH;\n+\t\t}\n+\t\tfprintf(stderr, \"%*s%s\\n\", pad + USAGE_GAP, \"\", subcmd->help);\n+\t}\n+\tfputc('\\n', stderr);\n+}\n+\n+int parse_subcommands(int argc, const char **argv, const char *prefix,\n+\t\tconst struct subcommand *subcmds, const char * const usagestr[])\n+{\n+\tconst struct subcommand *subcmd;\n+\n+\tfor (subcmd = subcmds; subcmd->name != NULL; subcmd ++)\n+\t\tif (!strcmp(subcmd->name, argv[0]))\n+\t\t\treturn (*subcmd->callback)(argc, argv, prefix);\n+\terror(\"unknown subcommand: %s\", argv[0]);\n+\tsubcommand_usage(usagestr, subcmds);\n+\texit(129);\n+}\ndiff --git a/parse-options.h b/parse-options.h\nindex 2e811dc..e95ad10 100644\n--- a/parse-options.h\n+++ b/parse-options.h\n@@ -46,6 +46,7 @@ enum parse_opt_option_flags {\n \n struct option;\n typedef int parse_opt_cb(const struct option *, const char *arg, int unset);\n+typedef int subcmd_cb(int argc, const char **argv, const char *prefix);\n \n struct parse_opt_ctx_t;\n typedef int parse_opt_ll_cb(struct parse_opt_ctx_t *ctx,\n@@ -117,6 +118,12 @@ struct option {\n \tintptr_t defval;\n };\n \n+struct subcommand {\n+\tconst char *name;\n+\tconst char *help;\n+\tsubcmd_cb *callback;\n+};\n+\n #define OPT_END()                   { OPTION_END }\n #define OPT_ARGUMENT(l, h)          { OPTION_ARGUMENT, 0, (l), NULL, NULL, \\\n \t\t\t\t      (h), PARSE_OPT_NOARG}\n@@ -169,6 +176,9 @@ extern int parse_options(int argc, const char **argv, const char *prefix,\n                          const struct option *options,\n                          const char * const usagestr[], int flags);\n \n+extern int parse_subcommands(int argc, const char **argv, const char *prefix,\n+\t\t\tconst struct subcommand *subcmds, const char * const usagestr[]);\n+\n extern NORETURN void usage_with_options(const char * const *usagestr,\n                                         const struct option *options);\n \n-- \n1.7.10\n"},{"id":"192816","messageId":"7vlik3t3j2.fsf@alter.siamese.dyndns.org","threadId":"30697","inReplyTo":"1338802534-32394-1-git-send-email-artagnon@gmail.com","subject":"Re: [RFC/PATCH] parse-options: introduce parse_subcommands","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-04T17:01:21Z","receivedAt":"2012-06-04T17:01:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n>  I found this patch in one of my branches from a long time ago.  I\n>  haven't written documentation and tests yet because I'm not fully\n>  convinced that I like this.\n>\n>  Thoughts?\n\nIt looks like a static version of string_list_lookup() and does not\nhave much reason to tie it to \"subcommand\".\n"},{"id":"192960","messageId":"20120605233257.GG3619@burratino","threadId":"30697","inReplyTo":"1338802534-32394-1-git-send-email-artagnon@gmail.com","subject":"Re: [RFC/PATCH] parse-options: introduce parse_subcommands","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-06-05T23:32:57Z","receivedAt":"2012-06-05T23:32:57Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nRamkumar Ramachandra wrote:\n\n>   $ git notes foo\n>   error: Unknown subcommand: foo\n>   usage: git notes [--ref <notes_ref>] [list [<object>]]\n>      or: git notes [--ref <notes_ref>] add [-f] [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]\n>      or: git notes [--ref <notes_ref>] copy [-f] <from-object> <to-object>\n[...]\n> is replaced by a more helpful:\n>\n>   $ git notes foo\n>   error: unknown subcommand: foo\n>   usage: git notes [<options>] [<subcommand>] [<options>] [<object>..]\n>\n>   available subcommands:\n>       list                  list notes for given object\n>       add                   add notes for given object\n>       copy                  copy notes for first object onto second object\n>       append                append notes to existing object\n>       edit                  edit notes for given object\n>       show                  show notes for given object\n>       merge                 merge given notes ref into current notes ref\n>       remove                remove notes for given objects\n>       prune                 remove all notes for non-existing/unreachable objects\n>       get-ref               print the current notes ref\n\nThe goal looks good.  As Junio said, the code looks simplifiable.\n\nHope that helps,\nJonathan\n"},{"id":"192994","messageId":"CALkWK0nuJ2VrAUD5PsfnZzO5yDgB+h4xn8g=CTqmUvJ8DOdPNQ@mail.gmail.com","threadId":"30697","inReplyTo":"7vlik3t3j2.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH] parse-options: introduce parse_subcommands","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2012-06-06T14:10:39Z","receivedAt":"2012-06-06T14:10:39Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi Junio,\n\nJunio C Hamano wrote:\n> Ramkumar Ramachandra <artagnon@gmail.com> writes:\n>\n>>  I found this patch in one of my branches from a long time ago.  I\n>>  haven't written documentation and tests yet because I'm not fully\n>>  convinced that I like this.\n>>\n>>  Thoughts?\n>\n> It looks like a static version of string_list_lookup() and does not\n> have much reason to tie it to \"subcommand\".\n\nHow so?  I can use a string_list to keep subcommand->name and stuff\nthe callback into util.  Where do I put subcommand->help then?\nI agree that there's no reason to tie it to subcommand though.\n\nRam\n"},{"id":"192998","messageId":"7vobowmjwr.fsf@alter.siamese.dyndns.org","threadId":"30697","inReplyTo":"CALkWK0nuJ2VrAUD5PsfnZzO5yDgB+h4xn8g=CTqmUvJ8DOdPNQ@mail.gmail.com","subject":"Re: [RFC/PATCH] parse-options: introduce parse_subcommands","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-06T17:26:12Z","receivedAt":"2012-06-06T17:26:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n>> It looks like a static version of string_list_lookup() and does not\n>> have much reason to tie it to \"subcommand\".\n>\n> How so?  I can use a string_list to keep subcommand->name and stuff\n> the callback into util.  Where do I put subcommand->help then?\n\nA string_list is a mapping from a string to an arbitrary piece of\ndata; there is nothing that stops you from placing a pointer to a\nstructure in its util field.\n\n> I agree that there's no reason to tie it to subcommand though.\n\nYeah, if it were a generic API for a mapping from a string to an\narbitrary piece of data that is determined at compile time, it would\nbe a useful addition, and at that point, it is misleading to call\nthat a \"parse-subcommand\" API.  It is just a look-up mechansim in\na fixed table keyed by strings.\n"},{"id":"193131","messageId":"1339145776-20495-1-git-send-email-artagnon@gmail.com","threadId":"30697","inReplyTo":"7vobowmjwr.fsf@alter.siamese.dyndns.org","subject":"[RFC] notes: attach help text to subcommands","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2012-06-08T08:56:16Z","receivedAt":"2012-06-08T08:56:16Z","isPatch":false,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Subcommands now have descriptive help text attached to them so that\nthe following error message:\n\n  $ git notes foo\n  error: Unknown subcommand: foo\n  usage: git notes [--ref <notes_ref>] [list [<object>]]\n     or: git notes [--ref <notes_ref>] add [-f] [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]\n     or: git notes [--ref <notes_ref>] copy [-f] <from-object> <to-object>\n     or: git notes [--ref <notes_ref>] append [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]\n     or: git notes [--ref <notes_ref>] edit [<object>]\n     or: git notes [--ref <notes_ref>] show [<object>]\n     or: git notes [--ref <notes_ref>] merge [-v | -q] [-s <strategy> ] <notes_ref>\n     or: git notes merge --commit [-v | -q]\n     or: git notes merge --abort [-v | -q]\n     or: git notes [--ref <notes_ref>] remove [<object>...]\n     or: git notes [--ref <notes_ref>] prune [-n | -v]\n     or: git notes [--ref <notes_ref>] get-ref\n\n    --ref <notes_ref>     use notes from <notes_ref>\n\nis replaced by a more helpful:\n\n  $ git notes foo\n  error: unknown subcommand: foo\n  usage: git notes [<options>] [<subcommand>] [<options>] [<object>..]\n\n  available subcommands:\n      list                  list notes for given object\n      add                   add notes for given object\n      copy                  copy notes for first object onto second object\n      append                append notes to existing object\n      edit                  edit notes for given object\n      show                  show notes for given object\n      merge                 merge given notes ref into current notes ref\n      remove                remove notes for given objects\n      prune                 remove all notes for non-existing/unreachable objects\n      get-ref               print the current notes ref\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n I coerced my patch to use the string_list API, and this is the\n result.  It looks terribly ugly, and I have no idea how to proceed.\n\n Thoughts/ suggestions?\n\n builtin/notes.c |  119 ++++++++++++++++++++++++++++++++++++++-----------------\n 1 file changed, 82 insertions(+), 37 deletions(-)\n\ndiff --git a/builtin/notes.c b/builtin/notes.c\nindex 3644d14..d5fcf45 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -20,18 +20,7 @@\n #include \"notes-merge.h\"\n \n static const char * const git_notes_usage[] = {\n-\t\"git notes [--ref <notes_ref>] [list [<object>]]\",\n-\t\"git notes [--ref <notes_ref>] add [-f] [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]\",\n-\t\"git notes [--ref <notes_ref>] copy [-f] <from-object> <to-object>\",\n-\t\"git notes [--ref <notes_ref>] append [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]\",\n-\t\"git notes [--ref <notes_ref>] edit [<object>]\",\n-\t\"git notes [--ref <notes_ref>] show [<object>]\",\n-\t\"git notes [--ref <notes_ref>] merge [-v | -q] [-s <strategy> ] <notes_ref>\",\n-\t\"git notes merge --commit [-v | -q]\",\n-\t\"git notes merge --abort [-v | -q]\",\n-\t\"git notes [--ref <notes_ref>] remove [<object>...]\",\n-\t\"git notes [--ref <notes_ref>] prune [-n | -v]\",\n-\t\"git notes [--ref <notes_ref>] get-ref\",\n+\t\"git notes [<options>] [<subcommand>] [<options>] [<object>..]\",\n \tNULL\n };\n \n@@ -1059,15 +1048,75 @@ static int get_ref(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n+\n+#define USAGE_OPTS_WIDTH 24\n+#define USAGE_GAP         2\n+\n+typedef int subcmd_cb(int argc, const char **argv, const char *prefix);\n+\n+struct subcommand {\n+\tconst char *help;\n+\tsubcmd_cb *callback;\n+};\n+\n+static void subcommand_usage(const char * const *usagestr,\n+\t\t\tstruct string_list *subcmds)\n+{\n+\tint i;\n+\n+\tfprintf(stderr, \"usage: %s\\n\", *usagestr++);\n+\twhile (*usagestr && **usagestr)\n+\t\tfprintf(stderr, \"   or: %s\\n\", *usagestr++);\n+\twhile (*usagestr) {\n+\t\tfprintf(stderr, \"%s%s\\n\",\n+\t\t\t\t**usagestr ? \"    \" : \"\",\n+\t\t\t\t*usagestr);\n+\t\tusagestr++;\n+\t}\n+\n+\tfputc('\\n', stderr);\n+\tfprintf(stderr, \"available subcommands:\\n\");\n+\tfor (i = 0; i < subcmds->nr; i++) {\n+\t\tsize_t pos;\n+\t\tint pad;\n+\n+\t\tpos = fprintf(stderr, \"    \");\n+\t\tpos += fprintf(stderr, \"%s\", subcmds->items[i].string);\n+\n+\t\tif (pos <= USAGE_OPTS_WIDTH)\n+\t\t\tpad = USAGE_OPTS_WIDTH - pos;\n+\t\telse {\n+\t\t\tfputc('\\n', stderr);\n+\t\t\tpad = USAGE_OPTS_WIDTH;\n+\t\t}\n+\t\tfprintf(stderr, \"%*s%s\\n\", pad + USAGE_GAP, \"\",\n+\t\t\t((struct subcommand *)(subcmds->items[i].util))->help);\n+\t}\n+\tfputc('\\n', stderr);\n+}\n+\n int cmd_notes(int argc, const char **argv, const char *prefix)\n {\n-\tint result;\n \tconst char *override_notes_ref = NULL;\n+\tstruct string_list subcmds = STRING_LIST_INIT_NODUP;\n+\tstruct string_list_item *item;\n \tstruct option options[] = {\n \t\tOPT_STRING(0, \"ref\", &override_notes_ref, \"notes_ref\",\n \t\t\t   \"use notes from <notes_ref>\"),\n \t\tOPT_END()\n \t};\n+\tstruct subcommand util[] = {\n+\t\t{ \"list notes for given object\", list },\n+\t\t{ \"add notes for given object\", add },\n+\t\t{ \"copy notes for first object onto second object\", copy },\n+\t\t{ \"append notes to existing object\", append_edit },\n+\t\t{ \"edit notes for given object\", append_edit },\n+\t\t{ \"show notes for given object\", show },\n+\t\t{ \"merge given notes ref into current notes ref\", merge },\n+\t\t{ \"remove notes for given objects\", remove_cmd },\n+\t\t{ \"remove all notes for non-existing/unreachable objects\", prune },\n+\t\t{ \"print the current notes ref\", get_ref }\n+\t};\n \n \tgit_config(git_default_config, NULL);\n \targc = parse_options(argc, argv, prefix, options, git_notes_usage,\n@@ -1081,28 +1130,24 @@ int cmd_notes(int argc, const char **argv, const char *prefix)\n \t\tstrbuf_release(&sb);\n \t}\n \n-\tif (argc < 1 || !strcmp(argv[0], \"list\"))\n-\t\tresult = list(argc, argv, prefix);\n-\telse if (!strcmp(argv[0], \"add\"))\n-\t\tresult = add(argc, argv, prefix);\n-\telse if (!strcmp(argv[0], \"copy\"))\n-\t\tresult = copy(argc, argv, prefix);\n-\telse if (!strcmp(argv[0], \"append\") || !strcmp(argv[0], \"edit\"))\n-\t\tresult = append_edit(argc, argv, prefix);\n-\telse if (!strcmp(argv[0], \"show\"))\n-\t\tresult = show(argc, argv, prefix);\n-\telse if (!strcmp(argv[0], \"merge\"))\n-\t\tresult = merge(argc, argv, prefix);\n-\telse if (!strcmp(argv[0], \"remove\"))\n-\t\tresult = remove_cmd(argc, argv, prefix);\n-\telse if (!strcmp(argv[0], \"prune\"))\n-\t\tresult = prune(argc, argv, prefix);\n-\telse if (!strcmp(argv[0], \"get-ref\"))\n-\t\tresult = get_ref(argc, argv, prefix);\n-\telse {\n-\t\tresult = error(_(\"Unknown subcommand: %s\"), argv[0]);\n-\t\tusage_with_options(git_notes_usage, options);\n-\t}\n-\n-\treturn result ? 1 : 0;\n+\tstring_list_insert(&subcmds, \"list\")->util = &util[0];\n+\tstring_list_insert(&subcmds, \"add\")->util = &util[1];\n+\tstring_list_insert(&subcmds, \"copy\")->util = &util[2];\n+\tstring_list_insert(&subcmds, \"append\")->util = &util[3];\n+\tstring_list_insert(&subcmds, \"edit\")->util = &util[4];\n+\tstring_list_insert(&subcmds, \"show\")->util = &util[5];\n+\tstring_list_insert(&subcmds, \"merge\")->util = &util[6];\n+\tstring_list_insert(&subcmds, \"remove\")->util = &util[7];\n+\tstring_list_insert(&subcmds, \"prune\")->util = &util[8];\n+\tstring_list_insert(&subcmds, \"get-ref\")->util = &util[9];\n+\n+\tif (argc < 1)\n+\t\treturn list(argc, argv, prefix) ? 1 : 0;\n+\n+\titem = unsorted_string_list_lookup(&subcmds, argv[0]);\n+\tif (item)\n+\t\treturn ((struct subcommand *)(item->util))->callback(argc, argv, prefix) ? 1 : 0;\n+\terror(\"unknown subcommand: %s\", argv[0]);\n+\tsubcommand_usage(git_notes_usage, &subcmds);\n+\texit(129);\n }\n-- \n1.7.10\n"},{"id":"193154","messageId":"7vd359eu4s.fsf@alter.siamese.dyndns.org","threadId":"30697","inReplyTo":"1339145776-20495-1-git-send-email-artagnon@gmail.com","subject":"Re: [RFC] notes: attach help text to subcommands","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-08T14:49:23Z","receivedAt":"2012-06-08T14:49:23Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n\n> +\tstring_list_insert(&subcmds, \"list\")->util = &util[0];\n> +\tstring_list_insert(&subcmds, \"add\")->util = &util[1];\n> ...\n\nIf you make \"struct subcommand\" to contain entry's own name, you\ncould simply do this, no?\n\n\tfor (i = 0; i < ARRAY_SIZE(util); i++)\n        \tstring_list_insert(&subcmds, util[i].name)->util = &util[i];\n\nPerhaps append all and then sort once at the end?\n"},{"id":"193162","messageId":"1339169317-12134-1-git-send-email-artagnon@gmail.com","threadId":"30697","inReplyTo":"7vd359eu4s.fsf@alter.siamese.dyndns.org","subject":"[PATCH] notes: attach help text to subcommands","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2012-06-08T15:28:37Z","receivedAt":"2012-06-08T15:28:37Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Attach descriptive help text to subcommands of 'git notes' so that the\nfollowing error message:\n\n  $ git notes foo\n  error: Unknown subcommand: foo\n  usage: git notes [--ref <notes_ref>] [list [<object>]]\n     or: git notes [--ref <notes_ref>] add [-f] [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]\n     or: git notes [--ref <notes_ref>] copy [-f] <from-object> <to-object>\n     or: git notes [--ref <notes_ref>] append [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]\n     or: git notes [--ref <notes_ref>] edit [<object>]\n     or: git notes [--ref <notes_ref>] show [<object>]\n     or: git notes [--ref <notes_ref>] merge [-v | -q] [-s <strategy> ] <notes_ref>\n     or: git notes merge --commit [-v | -q]\n     or: git notes merge --abort [-v | -q]\n     or: git notes [--ref <notes_ref>] remove [<object>...]\n     or: git notes [--ref <notes_ref>] prune [-n | -v]\n     or: git notes [--ref <notes_ref>] get-ref\n\n    --ref <notes_ref>     use notes from <notes_ref>\n\nis replaced by a more helpful:\n\n  $ git notes foo\n  error: unknown subcommand: foo\n  usage: git notes [<options>] [<subcommand>] [<options>] [<object>..]\n\n  available subcommands:\n      list                  list notes for given object\n      add                   add notes for given object\n      copy                  copy notes for first object onto second object\n      append                append notes to existing object\n      edit                  edit notes for given object\n      show                  show notes for given object\n      merge                 merge given notes ref into current notes ref\n      remove                remove notes for given objects\n      prune                 remove all notes for non-existing/unreachable objects\n      get-ref               print the current notes ref\n\nHelped-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n Junio C Hamano wrote:\n > If you make \"struct subcommand\" to contain entry's own name, you\n > could simply do this, no?\n >\n >        for (i = 0; i < ARRAY_SIZE(util); i++)\n >                string_list_insert(&subcmds, util[i].name)->util = &util[i];\n >\n > Perhaps append all and then sort once at the end?\n \n Thanks.  I wish we could do something aout USAGE_OPTS_WIDTH and\n USAGE_GAP; I stole them from parse-options.c.  Plus, `struct\n subcommand` and `subcommand_usage` need a new home if they're going\n to be used by more builtins with subcommands.\n \n Thoughts?\n\n builtin/notes.c |  112 +++++++++++++++++++++++++++++++++++++------------------\n 1 file changed, 76 insertions(+), 36 deletions(-)\n\ndiff --git a/builtin/notes.c b/builtin/notes.c\nindex 3644d14..223f990 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -20,18 +20,7 @@\n #include \"notes-merge.h\"\n \n static const char * const git_notes_usage[] = {\n-\t\"git notes [--ref <notes_ref>] [list [<object>]]\",\n-\t\"git notes [--ref <notes_ref>] add [-f] [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]\",\n-\t\"git notes [--ref <notes_ref>] copy [-f] <from-object> <to-object>\",\n-\t\"git notes [--ref <notes_ref>] append [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]\",\n-\t\"git notes [--ref <notes_ref>] edit [<object>]\",\n-\t\"git notes [--ref <notes_ref>] show [<object>]\",\n-\t\"git notes [--ref <notes_ref>] merge [-v | -q] [-s <strategy> ] <notes_ref>\",\n-\t\"git notes merge --commit [-v | -q]\",\n-\t\"git notes merge --abort [-v | -q]\",\n-\t\"git notes [--ref <notes_ref>] remove [<object>...]\",\n-\t\"git notes [--ref <notes_ref>] prune [-n | -v]\",\n-\t\"git notes [--ref <notes_ref>] get-ref\",\n+\t\"git notes [<options>] [<subcommand>] [<options>] [<object>..]\",\n \tNULL\n };\n \n@@ -1059,15 +1048,76 @@ static int get_ref(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n+\n+struct subcommand {\n+\tconst char *name;\n+\tconst char *help;\n+\tint (*callback)(int, const char **, const char *);\n+};\n+\n+#define USAGE_OPTS_WIDTH 24\n+#define USAGE_GAP         2\n+\n+static void subcommand_usage(const char * const *usagestr,\n+\t\t\tstruct string_list *subcmds)\n+{\n+\tint i;\n+\n+\tfprintf(stderr, \"usage: %s\\n\", *usagestr++);\n+\twhile (*usagestr && **usagestr)\n+\t\tfprintf(stderr, \"   or: %s\\n\", *usagestr++);\n+\twhile (*usagestr) {\n+\t\tfprintf(stderr, \"%s%s\\n\",\n+\t\t\t\t**usagestr ? \"    \" : \"\",\n+\t\t\t\t*usagestr);\n+\t\tusagestr++;\n+\t}\n+\n+\tfputc('\\n', stderr);\n+\tfprintf(stderr, \"available subcommands:\\n\");\n+\tfor (i = 0; i < subcmds->nr; i++) {\n+\t\tsize_t pos;\n+\t\tint pad;\n+\n+\t\tpos = fprintf(stderr, \"    \");\n+\t\tpos += fprintf(stderr, \"%s\", subcmds->items[i].string);\n+\n+\t\tif (pos <= USAGE_OPTS_WIDTH)\n+\t\t\tpad = USAGE_OPTS_WIDTH - pos;\n+\t\telse {\n+\t\t\tfputc('\\n', stderr);\n+\t\t\tpad = USAGE_OPTS_WIDTH;\n+\t\t}\n+\t\tfprintf(stderr, \"%*s%s\\n\", pad + USAGE_GAP, \"\",\n+\t\t\t((struct subcommand *)(subcmds->items[i].util))->help);\n+\t}\n+\tfputc('\\n', stderr);\n+}\n+\n int cmd_notes(int argc, const char **argv, const char *prefix)\n {\n-\tint result;\n+\tint i;\n \tconst char *override_notes_ref = NULL;\n+\tstruct string_list subcmds = STRING_LIST_INIT_NODUP;\n+\tstruct string_list_item *item;\n \tstruct option options[] = {\n \t\tOPT_STRING(0, \"ref\", &override_notes_ref, \"notes_ref\",\n \t\t\t   \"use notes from <notes_ref>\"),\n \t\tOPT_END()\n \t};\n+\tstruct subcommand util[] = {\n+\t\t{ \"list\", \"list notes for given object\", list },\n+\t\t{ \"add\", \"add notes for given object\", add },\n+\t\t{ \"copy\", \"copy notes for first object onto second object\", copy },\n+\t\t{ \"append\", \"append notes to existing object\", append_edit },\n+\t\t{ \"edit\", \"edit notes for given object\", append_edit },\n+\t\t{ \"show\", \"show notes for given object\", show },\n+\t\t{ \"merge\", \"merge given notes ref into current notes ref\", merge },\n+\t\t{ \"remove\", \"remove notes for given objects\", remove_cmd },\n+\t\t{ \"prune\",\n+\t\t  \"remove all notes for non-existing/unreachable objects\", prune },\n+\t\t{ \"get-ref\", \"print the current notes ref\", get_ref }\n+\t};\n \n \tgit_config(git_default_config, NULL);\n \targc = parse_options(argc, argv, prefix, options, git_notes_usage,\n@@ -1081,28 +1131,18 @@ int cmd_notes(int argc, const char **argv, const char *prefix)\n \t\tstrbuf_release(&sb);\n \t}\n \n-\tif (argc < 1 || !strcmp(argv[0], \"list\"))\n-\t\tresult = list(argc, argv, prefix);\n-\telse if (!strcmp(argv[0], \"add\"))\n-\t\tresult = add(argc, argv, prefix);\n-\telse if (!strcmp(argv[0], \"copy\"))\n-\t\tresult = copy(argc, argv, prefix);\n-\telse if (!strcmp(argv[0], \"append\") || !strcmp(argv[0], \"edit\"))\n-\t\tresult = append_edit(argc, argv, prefix);\n-\telse if (!strcmp(argv[0], \"show\"))\n-\t\tresult = show(argc, argv, prefix);\n-\telse if (!strcmp(argv[0], \"merge\"))\n-\t\tresult = merge(argc, argv, prefix);\n-\telse if (!strcmp(argv[0], \"remove\"))\n-\t\tresult = remove_cmd(argc, argv, prefix);\n-\telse if (!strcmp(argv[0], \"prune\"))\n-\t\tresult = prune(argc, argv, prefix);\n-\telse if (!strcmp(argv[0], \"get-ref\"))\n-\t\tresult = get_ref(argc, argv, prefix);\n-\telse {\n-\t\tresult = error(_(\"Unknown subcommand: %s\"), argv[0]);\n-\t\tusage_with_options(git_notes_usage, options);\n-\t}\n+\tfor (i = 0; i < ARRAY_SIZE(util); i++)\n+\t\tstring_list_insert(&subcmds, util[i].name)->util = &util[i];\n+\tsort_string_list(&subcmds);\n+\n+\tif (argc < 1)\n+\t\treturn list(argc, argv, prefix) ? 1 : 0;\n+\n+\titem = string_list_lookup(&subcmds, argv[0]);\n+\tif (item)\n+\t\treturn ((struct subcommand *)(item->util))->callback(argc, argv, prefix) ? 1 : 0;\n \n-\treturn result ? 1 : 0;\n+\terror(\"unknown subcommand: %s\", argv[0]);\n+\tsubcommand_usage(git_notes_usage, &subcmds);\n+\texit(129);\n }\n-- \n1.7.10\n"},{"id":"193164","messageId":"20120608153607.GC10380@burratino","threadId":"30697","inReplyTo":"1339169317-12134-1-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH] notes: attach help text to subcommands","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-06-08T15:36:07Z","receivedAt":"2012-06-08T15:36:07Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n>  Thanks.  I wish we could do something aout USAGE_OPTS_WIDTH and\n>  USAGE_GAP; I stole them from parse-options.\n\nExpose them in parse-options.h?  Or put this functionality in a\nparseopt-related file?\n\nCopy+paste must die. :)\n"},{"id":"193165","messageId":"CAMP44s28aoKkXyTr4Stow-empen9_CUxAWTDi_gcSUpKjMzg6Q@mail.gmail.com","threadId":"30697","inReplyTo":"20120608153607.GC10380@burratino","subject":"Re: [PATCH] notes: attach help text to subcommands","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-06-08T15:54:23Z","receivedAt":"2012-06-08T15:54:23Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Fri, Jun 8, 2012 at 5:36 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Ramkumar Ramachandra wrote:\n>\n>>  Thanks.  I wish we could do something aout USAGE_OPTS_WIDTH and\n>>  USAGE_GAP; I stole them from parse-options.\n>\n> Expose them in parse-options.h?  Or put this functionality in a\n> parseopt-related file?\n>\n> Copy+paste must die. :)\n\nThere's many commands that would benefit from this. In addition to\nprovide a better help for subcommands, this can be used to generate\nautomatically the shell completion list of subcommands.\n\nEven more; in zsh it would be possible to show this help text directly\nin the completion.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"193169","messageId":"CALkWK0k_tP7qGJ9zvAvrPtqMqAtK2QE7mibf2Yt1gssN10a1NA@mail.gmail.com","threadId":"30697","inReplyTo":"CAMP44s28aoKkXyTr4Stow-empen9_CUxAWTDi_gcSUpKjMzg6Q@mail.gmail.com","subject":"Re: [PATCH] notes: attach help text to subcommands","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2012-06-08T16:34:21Z","receivedAt":"2012-06-08T16:34:21Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi Jonathan and Felipe,\n\nJonathan Nieder wrote:\n> Ramkumar Ramachandra wrote:\n>\n>>  Thanks.  I wish we could do something aout USAGE_OPTS_WIDTH and\n>>  USAGE_GAP; I stole them from parse-options.\n>\n> Expose them in parse-options.h?  Or put this functionality in a\n> parseopt-related file?\n\nExposing a `subcommand_usage` in parse-options.h may not be a bad idea.\n\nFelipe Contreras wrote:\n> There's many commands that would benefit from this. In addition to\n> provide a better help for subcommands, this can be used to generate\n> automatically the shell completion list of subcommands.\n>\n> Even more; in zsh it would be possible to show this help text directly\n> in the completion.\n\nHow exactly is that going to work?  `git notes -h` doesn't output\nthis- so the only way to get this output is to invoke `git notes` with\nan invalid subcommand.  To fix that, I'd have to teach parse-options\nabout subcommands.\n\nRam\n"},{"id":"193172","messageId":"7vmx4dd9d6.fsf@alter.siamese.dyndns.org","threadId":"30697","inReplyTo":"1339169317-12134-1-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH] notes: attach help text to subcommands","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-08T17:03:17Z","receivedAt":"2012-06-08T17:03:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n> +struct subcommand {\n> +\tconst char *name;\n> +\tconst char *help;\n> +\tint (*callback)(int, const char **, const char *);\n> +};\n\nIn what way is this a \"callback\"?  It is not like you call some API\nfunction with this structure as its argument, and then the\nimplementation of the API function calls this function back.\n"},{"id":"193544","messageId":"CAMP44s0YrKJiLiinpScun4mJXC6W11SA=zEtAn5bWbLMnbFa5A@mail.gmail.com","threadId":"30697","inReplyTo":"CALkWK0k_tP7qGJ9zvAvrPtqMqAtK2QE7mibf2Yt1gssN10a1NA@mail.gmail.com","subject":"Re: [PATCH] notes: attach help text to subcommands","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-06-13T15:04:04Z","receivedAt":"2012-06-13T15:04:04Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Fri, Jun 8, 2012 at 6:34 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:\n> Hi Jonathan and Felipe,\n>\n> Jonathan Nieder wrote:\n>> Ramkumar Ramachandra wrote:\n>>\n>>>  Thanks.  I wish we could do something aout USAGE_OPTS_WIDTH and\n>>>  USAGE_GAP; I stole them from parse-options.\n>>\n>> Expose them in parse-options.h?  Or put this functionality in a\n>> parseopt-related file?\n>\n> Exposing a `subcommand_usage` in parse-options.h may not be a bad idea.\n>\n> Felipe Contreras wrote:\n>> There's many commands that would benefit from this. In addition to\n>> provide a better help for subcommands, this can be used to generate\n>> automatically the shell completion list of subcommands.\n>>\n>> Even more; in zsh it would be possible to show this help text directly\n>> in the completion.\n>\n> How exactly is that going to work?  `git notes -h` doesn't output\n> this- so the only way to get this output is to invoke `git notes` with\n> an invalid subcommand.  To fix that, I'd have to teach parse-options\n> about subcommands.\n\nYeah, something like that. See this proposal:\n\nhttp://mid.gmane.org/1334140165-24958-2-git-send-email-bebarino@gmail.com\n\nI would prefer something like 'git info --subcommands notes' or\nsomething like that.\n\nCheers.\n\n-- \nFelipe Contreras\n"}]}