git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH] builtin-notes: Minor (mostly parse_options-related) fixes

From
Johan Herland <johan@herland.net>
Date
Feb 25, 2010, 00:48 UTC
Message-ID
<201002250148.11487.johan@herland.net>
In-Reply-To
<201002160247.49719.johan@herland.net>

Use PARSE_OPT_NONEG to disallow --no-<option> for --message, --file, --reedit-message and --reuse-message. --no-<option> does not make sense for these options, and specifying PARSE_OPT_NONEG simplifies the code in the option-handling callbacks.

Also, use strbuf_addch(... '\n') instead of strbuf_addstr(... "\n") in couple of places.

Finally, improve the short-help by dividing the options into two OPT_GROUPs.
Suggested-by: Stephen Boyd <bebarino@gmail.com>
Signed-off-by: Johan Herland <johan@herland.net>
---
On Tuesday 16 February 2010, Johan Herland wrote:
> On Monday 15 February 2010, Stephen Boyd wrote:
> > On 02/13/2010 01:28 PM, Johan Herland wrote:
> > > @@ -199,6 +203,40 @@ static int parse_file_arg(const struct option
[...]
Show 20 quoted lines
> > > +	if (!arg)
> > > +		return -1;
> > 
> > This is impossible unless you're using the PARSE_OPT_OPTARG flag or
> > allowing negation (i.e. --no-reuse-mesage). You should probably make
> > the two callback options PARSE_OPT_NONEG and then drop this if
> > statement. Same applies to some of the other callbacks not introduced
> > in this patch.
> 
> Thanks. Fixed locally. Will be part of the next iteration.
> 
> > > +
> > > +	if (msg->buf.len)
> > > +		strbuf_addstr(&(msg->buf), "\n");
> > > +
> > 
> > Use strbuf_addch()? I saw this in a couple other patches too.
> 
> Of course. Thanks for noticing. Fixed locally. Will be part of the next
> iteration.

I never got around to submitting the next jh/notes iteration before it was merged to 'next', so here are Stephen's suggestions as a separate patch on top of 'next'. AFAICS it does not conflict with tr/notes-display in 'pu'.

Have fun! :)
...Johan
 builtin-notes.c |   38 +++++++++++++++++---------------------
 1 files changed, 17 insertions(+), 21 deletions(-)
diff --git a/builtin-notes.c b/builtin-notes.c
index 123ecad..feb710a 100644
--- a/builtin-notes.c
+++ b/builtin-notes.c
@@ -171,12 +171,9 @@ static int parse_msg_arg(const struct option *opt, const char *arg, int unset)
 {
 	struct msg_arg *msg = opt->value;
 
-	if (!arg)
-		return -1;
-
 	strbuf_grow(&(msg->buf), strlen(arg) + 2);
 	if (msg->buf.len)
-		strbuf_addstr(&(msg->buf), "\n");
+		strbuf_addch(&(msg->buf), '\n');
 	strbuf_addstr(&(msg->buf), arg);
 	stripspace(&(msg->buf), 0);
 
@@ -188,11 +185,8 @@ static int parse_file_arg(const struct option *opt, const char *arg, int unset)
 {
 	struct msg_arg *msg = opt->value;
 
-	if (!arg)
-		return -1;
-
 	if (msg->buf.len)
-		strbuf_addstr(&(msg->buf), "\n");
+		strbuf_addch(&(msg->buf), '\n');
 	if (!strcmp(arg, "-")) {
 		if (strbuf_read(&(msg->buf), 0, 1024) < 0)
 			die_errno("cannot read '%s'", arg);
@@ -212,11 +206,8 @@ static int parse_reuse_arg(const struct option *opt, const char *arg, int unset)
 	enum object_type type;
 	unsigned long len;
 
-	if (!arg)
-		return -1;
-
 	if (msg->buf.len)
-		strbuf_addstr(&(msg->buf), "\n");
+		strbuf_addch(&(msg->buf), '\n');
 
 	if (get_sha1(arg, object))
 		die("Failed to resolve '%s' as a valid ref.", arg);
@@ -291,15 +282,20 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 	int given_object = 0, i = 1, retval = 0;
 	struct msg_arg msg = { 0, 0, STRBUF_INIT };
 	struct option options[] = {
-		OPT_GROUP("Notes options"),
-		OPT_CALLBACK('m', "message", &msg, "MSG",
-			     "note contents as a string", parse_msg_arg),
-		OPT_CALLBACK('F', "file", &msg, "FILE",
-			     "note contents in a file", parse_file_arg),
-		OPT_CALLBACK('c', "reedit-message", &msg, "OBJECT",
-			   "reuse and edit specified note object", parse_reedit_arg),
-		OPT_CALLBACK('C', "reuse-message", &msg, "OBJECT",
-			   "reuse specified note object", parse_reuse_arg),
+		OPT_GROUP("Notes contents options"),
+		{ OPTION_CALLBACK, 'm', "message", &msg, "MSG",
+			"note contents as a string", PARSE_OPT_NONEG,
+			parse_msg_arg},
+		{ OPTION_CALLBACK, 'F', "file", &msg, "FILE",
+			"note contents in a file", PARSE_OPT_NONEG,
+			parse_file_arg},
+		{ OPTION_CALLBACK, 'c', "reedit-message", &msg, "OBJECT",
+			"reuse and edit specified note object", PARSE_OPT_NONEG,
+			parse_reedit_arg},
+		{ OPTION_CALLBACK, 'C', "reuse-message", &msg, "OBJECT",
+			"reuse specified note object", PARSE_OPT_NONEG,
+			parse_reuse_arg},
+		OPT_GROUP("Other options"),
 		OPT_BOOLEAN('f', "force", &force, "replace existing notes"),
 		OPT_END()
 	};

-- 
1.7.0.rc1.141.gd3fd

-- 
Johan Herland, <johan@herland.net>
www.herland.net
Previous: Johan HerlandNext: Stephen Boyd
Message 32 of 35 in “[PATCHv13 00/30] git notes”
  1. Johan HerlandFeb 13, 2010
  2. 01/30 Minor cosmetic fixes to notes.cJohan Herland, Feb 13, 2010
  3. 02/30 Notes API: get_commit_notes() -> format_note() + remove the commit restrictionJohan Herland, Feb 13, 2010
  4. 03/30 Add tests for checking correct handling of $GIT_NOTES_REF and core.notesRefJohan Herland, Feb 13, 2010
  5. 04/30 Notes API: init_notes(): Initialize the notes tree from the given notes refJohan Herland, Feb 13, 2010
  6. 05/30 Notes API: add_note(): Add note objects to the internal notes tree structureJohan Herland, Feb 13, 2010
  7. 06/30 Notes API: remove_note(): Remove note objects from the notes tree structureJohan Herland, Feb 13, 2010
  8. 07/30 Notes API: get_note(): Return the note annotating the given objectJohan Herland, Feb 13, 2010
  9. 08/30 Notes API: for_each_note(): Traverse the entire notes tree with a callbackJohan Herland, Feb 13, 2010
  10. 09/30 Notes API: write_notes_tree(): Store the notes tree in the databaseJohan Herland, Feb 13, 2010
  11. 10/30 Notes API: Allow multiple concurrent notes trees with new struct notes_treeJohan Herland, Feb 13, 2010
  12. 11/30 Refactor notes concatenation into a flexible interface for combining notesJohan Herland, Feb 13, 2010
  13. 12/30 Builtin-ify git-notesJohan Herland, Feb 13, 2010
  14. 13/30 t3301: Verify successful annotation of non-commitsJohan Herland, Feb 13, 2010
  15. 14/30 t3305: Verify that adding many notes with git-notes triggers increased fanoutJohan Herland, Feb 13, 2010
  16. 15/30 Teach notes code to properly preserve non-notes in the notes treeJohan Herland, Feb 13, 2010
  17. 16/30 Teach builtin-notes to remove empty notesJohan Herland, Feb 13, 2010
  18. 17/30 builtin-notes: Add "remove" subcommand for removing existing notesJohan Herland, Feb 13, 2010
  19. 18/30 t3305: Verify that removing notes triggers automatic fanout consolidationJohan Herland, Feb 13, 2010
  20. 19/30 Notes API: prune_notes(): Prune notes that belong to non-existing objectsJohan Herland, Feb 13, 2010
  21. 20/30 builtin-notes: Add "prune" subcommand for removing notes for missing objectsJohan Herland, Feb 13, 2010
  22. 21/30 Documentation: Generalize git-notes docs to 'objects' instead of 'commits'Johan Herland, Feb 13, 2010
  23. 22/30 builtin-notes: Add "list" subcommand for listing note objectsJohan Herland, Feb 13, 2010
  24. 23/30 builtin-notes: Add --message/--file aliases for -m/-F optionsJohan Herland, Feb 13, 2010
  25. 24/30 builtin-notes: Add "add" subcommand for adding notes to objectsJohan Herland, Feb 13, 2010
  26. 25/30 builtin-notes: Add "append" subcommand for appending to note objectsJohan Herland, Feb 13, 2010
  27. 26/30 builtin-notes: Deprecate the -m/-F options for "git notes edit"Johan Herland, Feb 13, 2010
  28. 27/30 builtin-notes: Refactor handling of -F option to allow combining -m and -FJohan Herland, Feb 13, 2010
  29. 28/30 builtin-notes: Add -c/-C options for reusing notesJohan Herland, Feb 13, 2010
  30. Stephen BoydFeb 15, 2010
  31. Johan HerlandFeb 16, 2010
  32. builtin-notes: Minor (mostly parse_options-related) fixesJohan Herland, Feb 25, 2010
  33. Stephen BoydFeb 25, 2010
  34. 29/30 builtin-notes: Misc. refactoring of argc and exit value handlingJohan Herland, Feb 13, 2010
  35. 30/30 builtin-notes: Add "copy" subcommand for copying notes between objectsJohan Herland, Feb 13, 2010

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.