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

[PATCH] commit-tree: utilize parse-options api

From
Brandon <brandon1024.br@gmail.com>
Date
Feb 26, 2019, 20:09 UTC
Message-ID
<20190226200952.33950-1-brandon1024.br@gmail.com>
From: Brandon Richardson <brandon1024.br@gmail.com>

Rather than parse options manually, which is both difficult to read and error prone, parse options supplied to commit-tree using the parse-options api.

It was discovered that the --no-gpg-sign option was documented but not implemented in 55ca3f99, and the existing implementation would attempt to translate the option as a tree oid.It was also suggested in 55ca3f99 that commit-tree should be migrated to utilize the parse-options api, which could help prevent mistakes like this in the future. Hence this change.

Signed-off-by: Brandon Richardson <brandon1024.br@gmail.com>
---
Notes:
    GitHub Pull Request: https://github.com/brandon1024/git/pull/1
    Travis CI Results: https://travis-ci.com/brandon1024/git/builds/102337393
 builtin/commit-tree.c | 162 ++++++++++++++++++++++++------------------
 1 file changed, 92 insertions(+), 70 deletions(-)
diff --git a/builtin/commit-tree.c b/builtin/commit-tree.c
index 12cc403bd7..310f38d000 100644
--- a/builtin/commit-tree.c
+++ b/builtin/commit-tree.c
@@ -12,8 +12,14 @@
 #include "builtin.h"
 #include "utf8.h"
 #include "gpg-interface.h"
+#include "parse-options.h"
+#include "string-list.h"
 
-static const char commit_tree_usage[] = "git commit-tree [(-p <sha1>)...] [-S[<keyid>]] [-m <message>] [-F <file>] <sha1>";
+static const char * const builtin_commit_tree_usage[] = {
+	N_("git commit-tree [(-p <parent>)...] [-S[<keyid>]] [(-m <message>)...] "
+		"[(-F <file>)...] <tree>"),
+	NULL
+};
 
 static const char *sign_commit;
 
@@ -39,87 +45,103 @@ static int commit_tree_config(const char *var, const char *value, void *cb)
 	return git_default_config(var, value, cb);
 }
 
+static int parse_parent_arg_callback(const struct option *opt,
+		const char *arg, int unset)
+{
+	struct object_id oid;
+	struct commit_list **parents = opt->value;
+
+	BUG_ON_OPT_NEG(unset);
+
+	if (!arg)
+		return 1;
+	if (get_oid_commit(arg, &oid))
+		die("Not a valid object name %s", arg);
+
+	assert_oid_type(&oid, OBJ_COMMIT);
+	new_parent(lookup_commit(the_repository, &oid), parents);
+	return 0;
+}
+
+static int parse_message_arg_callback(const struct option *opt,
+		const char *arg, int unset)
+{
+	struct strbuf *buf = opt->value;
+
+	BUG_ON_OPT_NEG(unset);
+
+	if (!arg)
+		return 1;
+	if (buf->len)
+		strbuf_addch(buf, '\n');
+	strbuf_addstr(buf, arg);
+	strbuf_complete_line(buf);
+
+	return 0;
+}
+
+static int parse_file_arg_callback(const struct option *opt,
+		const char *arg, int unset)
+{
+	int fd;
+	struct strbuf *buf = opt->value;
+
+	BUG_ON_OPT_NEG(unset);
+
+	if (!arg)
+		return 1;
+	if (buf->len)
+		strbuf_addch(buf, '\n');
+	if (!strcmp(arg, "-"))
+		fd = 0;
+	else {
+		fd = open(arg, O_RDONLY);
+		if (fd < 0)
+			die_errno("git commit-tree: failed to open '%s'", arg);
+	}
+	if (strbuf_read(buf, fd, 0) < 0)
+		die_errno("git commit-tree: failed to read '%s'", arg);
+	if (fd && close(fd))
+		die_errno("git commit-tree: failed to close '%s'", arg);
+
+	return 0;
+}
+
 int cmd_commit_tree(int argc, const char **argv, const char *prefix)
 {
-	int i, got_tree = 0;
+	static struct strbuf buffer = STRBUF_INIT;
 	struct commit_list *parents = NULL;
 	struct object_id tree_oid;
 	struct object_id commit_oid;
-	struct strbuf buffer = STRBUF_INIT;
+
+    struct option builtin_commit_tree_options[] = {
+		{ OPTION_CALLBACK, 'p', NULL, &parents, "parent",
+		  N_("id of a parent commit object"), PARSE_OPT_NONEG,
+		  parse_parent_arg_callback },
+		{ OPTION_CALLBACK, 'm', NULL, &buffer, N_("message"),
+		  N_("commit message"), PARSE_OPT_NONEG,
+		  parse_message_arg_callback },
+		{ OPTION_CALLBACK, 'F', NULL, &buffer, N_("file"),
+		  N_("read commit log message from file"), PARSE_OPT_NONEG,
+		  parse_file_arg_callback },
+		{ OPTION_STRING, 'S', "gpg-sign", &sign_commit, N_("key-id"),
+		  N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
+		OPT_END()
+    };
 
 	git_config(commit_tree_config, NULL);
 
 	if (argc < 2 || !strcmp(argv[1], "-h"))
-		usage(commit_tree_usage);
-
-	for (i = 1; i < argc; i++) {
-		const char *arg = argv[i];
-		if (!strcmp(arg, "-p")) {
-			struct object_id oid;
-			if (argc <= ++i)
-				usage(commit_tree_usage);
-			if (get_oid_commit(argv[i], &oid))
-				die("Not a valid object name %s", argv[i]);
-			assert_oid_type(&oid, OBJ_COMMIT);
-			new_parent(lookup_commit(the_repository, &oid),
-						 &parents);
-			continue;
-		}
+		usage_with_options(builtin_commit_tree_usage, builtin_commit_tree_options);
 
-		if (!strcmp(arg, "--gpg-sign")) {
-		    sign_commit = "";
-		    continue;
-		}
-
-		if (skip_prefix(arg, "-S", &sign_commit) ||
-			skip_prefix(arg, "--gpg-sign=", &sign_commit))
-			continue;
-
-		if (!strcmp(arg, "--no-gpg-sign")) {
-			sign_commit = NULL;
-			continue;
-		}
+	argc = parse_options(argc, argv, prefix, builtin_commit_tree_options,
+			builtin_commit_tree_usage, 0);
 
-		if (!strcmp(arg, "-m")) {
-			if (argc <= ++i)
-				usage(commit_tree_usage);
-			if (buffer.len)
-				strbuf_addch(&buffer, '\n');
-			strbuf_addstr(&buffer, argv[i]);
-			strbuf_complete_line(&buffer);
-			continue;
-		}
-
-		if (!strcmp(arg, "-F")) {
-			int fd;
-
-			if (argc <= ++i)
-				usage(commit_tree_usage);
-			if (buffer.len)
-				strbuf_addch(&buffer, '\n');
-			if (!strcmp(argv[i], "-"))
-				fd = 0;
-			else {
-				fd = open(argv[i], O_RDONLY);
-				if (fd < 0)
-					die_errno("git commit-tree: failed to open '%s'",
-						  argv[i]);
-			}
-			if (strbuf_read(&buffer, fd, 0) < 0)
-				die_errno("git commit-tree: failed to read '%s'",
-					  argv[i]);
-			if (fd && close(fd))
-				die_errno("git commit-tree: failed to close '%s'",
-					  argv[i]);
-			continue;
-		}
+	if (argc != 1)
+		die("Must give exactly one tree");
 
-		if (get_oid_tree(arg, &tree_oid))
-			die("Not a valid object name %s", arg);
-		if (got_tree)
-			die("Cannot give more than one trees");
-		got_tree = 1;
-	}
+	if (get_oid_tree(argv[0], &tree_oid))
+		die("Not a valid object name %s", argv[0]);
 
 	if (!buffer.len) {
 		if (strbuf_read(&buffer, 0, 0) < 0)
-- 
2.21.0
Next: Andrei Rybak
Message 1 of 14 in “commit-tree: utilize parse-options api”
  1. commit-tree: utilize parse-options apiBrandon, Feb 26, 2019
  2. Andrei RybakFeb 26, 2019
  3. Brandon RichardsonFeb 26, 2019
  4. Duy NguyenFeb 27, 2019
  5. Duy NguyenFeb 27, 2019
  6. SZEDER GáborFeb 27, 2019
  7. Duy NguyenFeb 27, 2019
  8. SZEDER GáborFeb 27, 2019
  9. Duy NguyenFeb 28, 2019
  10. Brandon RichardsonFeb 27, 2019
  11. Duy NguyenFeb 28, 2019
  12. Jeff KingFeb 27, 2019
  13. Brandon RichardsonFeb 28, 2019
  14. Jeff KingFeb 28, 2019

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.