Re: [RFC] notes: attach help text to subcommands

Ramkumar Ramachandra [off-list ref] writes:

+	string_list_insert(&subcmds, "list")->util = &util[0];
+	string_list_insert(&subcmds, "add")->util = &util[1];
...

If you make "struct subcommand" to contain entry's own name, you
could simply do this, no?

	for (i = 0; i < ARRAY_SIZE(util); i++)
        	string_list_insert(&subcmds, util[i].name)->util = &util[i];

Perhaps append all and then sort once at the end?

Attach descriptive help text to subcommands of 'git notes' so that the
following error message:

  $ git notes foo
  error: Unknown subcommand: foo
  usage: git notes [--ref <notes_ref>] [list [<object>]]
     or: git notes [--ref <notes_ref>] add [-f] [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]
     or: git notes [--ref <notes_ref>] copy [-f] <from-object> <to-object>
     or: git notes [--ref <notes_ref>] append [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]
     or: git notes [--ref <notes_ref>] edit [<object>]
     or: git notes [--ref <notes_ref>] show [<object>]
     or: git notes [--ref <notes_ref>] merge [-v | -q] [-s <strategy> ] <notes_ref>
     or: git notes merge --commit [-v | -q]
     or: git notes merge --abort [-v | -q]
     or: git notes [--ref <notes_ref>] remove [<object>...]
     or: git notes [--ref <notes_ref>] prune [-n | -v]
     or: git notes [--ref <notes_ref>] get-ref

    --ref <notes_ref>     use notes from <notes_ref>

is replaced by a more helpful:

  $ git notes foo
  error: unknown subcommand: foo
  usage: git notes [<options>] [<subcommand>] [<options>] [<object>..]

  available subcommands:
      list                  list notes for given object
      add                   add notes for given object
      copy                  copy notes for first object onto second object
      append                append notes to existing object
      edit                  edit notes for given object
      show                  show notes for given object
      merge                 merge given notes ref into current notes ref
      remove                remove notes for given objects
      prune                 remove all notes for non-existing/unreachable objects
      get-ref               print the current notes ref

Helped-by: Junio C Hamano [off-list ref]
Signed-off-by: Ramkumar Ramachandra <redacted>
---
 Junio C Hamano wrote:
 > If you make "struct subcommand" to contain entry's own name, you
 > could simply do this, no?
 >
 >        for (i = 0; i < ARRAY_SIZE(util); i++)
 >                string_list_insert(&subcmds, util[i].name)->util = &util[i];
 >
 > Perhaps append all and then sort once at the end?
 
 Thanks.  I wish we could do something aout USAGE_OPTS_WIDTH and
 USAGE_GAP; I stole them from parse-options.c.  Plus, `struct
 subcommand` and `subcommand_usage` need a new home if they're going
 to be used by more builtins with subcommands.
 
 Thoughts?

 builtin/notes.c |  112 +++++++++++++++++++++++++++++++++++++------------------
 1 file changed, 76 insertions(+), 36 deletions(-)

diff --git a/builtin/notes.c b/builtin/notes.c
index 3644d14..223f990 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c

@@ -20,18 +20,7 @@
 #include "notes-merge.h"
 
 static const char * const git_notes_usage[] = {
-	"git notes [--ref <notes_ref>] [list [<object>]]",
-	"git notes [--ref <notes_ref>] add [-f] [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]",
-	"git notes [--ref <notes_ref>] copy [-f] <from-object> <to-object>",
-	"git notes [--ref <notes_ref>] append [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]",
-	"git notes [--ref <notes_ref>] edit [<object>]",
-	"git notes [--ref <notes_ref>] show [<object>]",
-	"git notes [--ref <notes_ref>] merge [-v | -q] [-s <strategy> ] <notes_ref>",
-	"git notes merge --commit [-v | -q]",
-	"git notes merge --abort [-v | -q]",
-	"git notes [--ref <notes_ref>] remove [<object>...]",
-	"git notes [--ref <notes_ref>] prune [-n | -v]",
-	"git notes [--ref <notes_ref>] get-ref",
+	"git notes [<options>] [<subcommand>] [<options>] [<object>..]",
 	NULL
 };
 

@@ -1059,15 +1048,76 @@ static int get_ref(int argc, const char **argv, const char *prefix)
 	return 0;
 }
 
+
+struct subcommand {
+	const char *name;
+	const char *help;
+	int (*callback)(int, const char **, const char *);
+};
+
+#define USAGE_OPTS_WIDTH 24
+#define USAGE_GAP         2
+
+static void subcommand_usage(const char * const *usagestr,
+			struct string_list *subcmds)
+{
+	int i;
+
+	fprintf(stderr, "usage: %s\n", *usagestr++);
+	while (*usagestr && **usagestr)
+		fprintf(stderr, "   or: %s\n", *usagestr++);
+	while (*usagestr) {
+		fprintf(stderr, "%s%s\n",
+				**usagestr ? "    " : "",
+				*usagestr);
+		usagestr++;
+	}
+
+	fputc('\n', stderr);
+	fprintf(stderr, "available subcommands:\n");
+	for (i = 0; i < subcmds->nr; i++) {
+		size_t pos;
+		int pad;
+
+		pos = fprintf(stderr, "    ");
+		pos += fprintf(stderr, "%s", subcmds->items[i].string);
+
+		if (pos <= USAGE_OPTS_WIDTH)
+			pad = USAGE_OPTS_WIDTH - pos;
+		else {
+			fputc('\n', stderr);
+			pad = USAGE_OPTS_WIDTH;
+		}
+		fprintf(stderr, "%*s%s\n", pad + USAGE_GAP, "",
+			((struct subcommand *)(subcmds->items[i].util))->help);
+	}
+	fputc('\n', stderr);
+}
+
 int cmd_notes(int argc, const char **argv, const char *prefix)
 {
-	int result;
+	int i;
 	const char *override_notes_ref = NULL;
+	struct string_list subcmds = STRING_LIST_INIT_NODUP;
+	struct string_list_item *item;
 	struct option options[] = {
 		OPT_STRING(0, "ref", &override_notes_ref, "notes_ref",
 			   "use notes from <notes_ref>"),
 		OPT_END()
 	};
+	struct subcommand util[] = {
+		{ "list", "list notes for given object", list },
+		{ "add", "add notes for given object", add },
+		{ "copy", "copy notes for first object onto second object", copy },
+		{ "append", "append notes to existing object", append_edit },
+		{ "edit", "edit notes for given object", append_edit },
+		{ "show", "show notes for given object", show },
+		{ "merge", "merge given notes ref into current notes ref", merge },
+		{ "remove", "remove notes for given objects", remove_cmd },
+		{ "prune",
+		  "remove all notes for non-existing/unreachable objects", prune },
+		{ "get-ref", "print the current notes ref", get_ref }
+	};
 
 	git_config(git_default_config, NULL);
 	argc = parse_options(argc, argv, prefix, options, git_notes_usage,

@@ -1081,28 +1131,18 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 		strbuf_release(&sb);
 	}
 
-	if (argc < 1 || !strcmp(argv[0], "list"))
-		result = list(argc, argv, prefix);
-	else if (!strcmp(argv[0], "add"))
-		result = add(argc, argv, prefix);
-	else if (!strcmp(argv[0], "copy"))
-		result = copy(argc, argv, prefix);
-	else if (!strcmp(argv[0], "append") || !strcmp(argv[0], "edit"))
-		result = append_edit(argc, argv, prefix);
-	else if (!strcmp(argv[0], "show"))
-		result = show(argc, argv, prefix);
-	else if (!strcmp(argv[0], "merge"))
-		result = merge(argc, argv, prefix);
-	else if (!strcmp(argv[0], "remove"))
-		result = remove_cmd(argc, argv, prefix);
-	else if (!strcmp(argv[0], "prune"))
-		result = prune(argc, argv, prefix);
-	else if (!strcmp(argv[0], "get-ref"))
-		result = get_ref(argc, argv, prefix);
-	else {
-		result = error(_("Unknown subcommand: %s"), argv[0]);
-		usage_with_options(git_notes_usage, options);
-	}
+	for (i = 0; i < ARRAY_SIZE(util); i++)
+		string_list_insert(&subcmds, util[i].name)->util = &util[i];
+	sort_string_list(&subcmds);
+
+	if (argc < 1)
+		return list(argc, argv, prefix) ? 1 : 0;
+
+	item = string_list_lookup(&subcmds, argv[0]);
+	if (item)
+		return ((struct subcommand *)(item->util))->callback(argc, argv, prefix) ? 1 : 0;
 
-	return result ? 1 : 0;
+	error("unknown subcommand: %s", argv[0]);
+	subcommand_usage(git_notes_usage, &subcmds);
+	exit(129);
 }

-- 
1.7.10

Ramkumar Ramachandra wrote:

 Thanks.  I wish we could do something aout USAGE_OPTS_WIDTH and
 USAGE_GAP; I stole them from parse-options.

Expose them in parse-options.h?  Or put this functionality in a
parseopt-related file?

Copy+paste must die. :)

On Fri, Jun 8, 2012 at 5:36 PM, Jonathan Nieder [off-list ref] wrote:

Ramkumar Ramachandra wrote:

quoted

 Thanks.  I wish we could do something aout USAGE_OPTS_WIDTH and
 USAGE_GAP; I stole them from parse-options.

Expose them in parse-options.h?  Or put this functionality in a
parseopt-related file?

Copy+paste must die. :)

There's many commands that would benefit from this. In addition to
provide a better help for subcommands, this can be used to generate
automatically the shell completion list of subcommands.

Even more; in zsh it would be possible to show this help text directly
in the completion.

Cheers.

-- 
Felipe Contreras

Hi Jonathan and Felipe,

Jonathan Nieder wrote:

Ramkumar Ramachandra wrote:

quoted

 Thanks.  I wish we could do something aout USAGE_OPTS_WIDTH and
 USAGE_GAP; I stole them from parse-options.

Expose them in parse-options.h?  Or put this functionality in a
parseopt-related file?

Exposing a `subcommand_usage` in parse-options.h may not be a bad idea.

Felipe Contreras wrote:

There's many commands that would benefit from this. In addition to
provide a better help for subcommands, this can be used to generate
automatically the shell completion list of subcommands.

Even more; in zsh it would be possible to show this help text directly
in the completion.

How exactly is that going to work?  `git notes -h` doesn't output
this- so the only way to get this output is to invoke `git notes` with
an invalid subcommand.  To fix that, I'd have to teach parse-options
about subcommands.

Ram

On Fri, Jun 8, 2012 at 6:34 PM, Ramkumar Ramachandra [off-list ref] wrote:

Hi Jonathan and Felipe,

Jonathan Nieder wrote:

quoted

Ramkumar Ramachandra wrote:

quoted

 Thanks.  I wish we could do something aout USAGE_OPTS_WIDTH and
 USAGE_GAP; I stole them from parse-options.

Expose them in parse-options.h?  Or put this functionality in a
parseopt-related file?

Exposing a `subcommand_usage` in parse-options.h may not be a bad idea.

Felipe Contreras wrote:

quoted

There's many commands that would benefit from this. In addition to
provide a better help for subcommands, this can be used to generate
automatically the shell completion list of subcommands.

Even more; in zsh it would be possible to show this help text directly
in the completion.

How exactly is that going to work?  `git notes -h` doesn't output
this- so the only way to get this output is to invoke `git notes` with
an invalid subcommand.  To fix that, I'd have to teach parse-options
about subcommands.

Yeah, something like that. See this proposal:

http://mid.gmane.org/1334140165-24958-2-git-send-email-bebarino@gmail.com

I would prefer something like 'git info --subcommands notes' or
something like that.

Cheers.

-- 
Felipe Contreras