commit-tree: utilize parse-options api

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 commit 70ddbd7767 (commit-tree: add missing --gpg-sign flag, 2019-01-19), and the existing implementation would attempt to translate the option as a tree oid. It was also suggested earlier in commit 55ca3f99ae (commit-tree: add and document --no-gpg-sign, 2013-12-13) 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. Also update the documentation to better describe that mixing `-m` and `-F` options will correctly compose commit log messages in the order in which the options are given. In the process, mark various strings for translation. Signed-off-by: Brandon Richardson <brandon1024.br@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Brandon Richardson committed Mar 7, 2019 at 11:44 UTC cbdeab98e871bfbb45cf5095cc804ebb07054eba
3 files changed +104 -74
Documentation/git-commit-tree.txt
+7 -2
@@ -23,6 +23,10 @@ Creates a new commit object based on the provided tree object and
23 emits the new commit object id on stdout. The log message is read
24 from the standard input, unless `-m` or `-F` options are given.
25
26 +The `-m` and `-F` options can be given any number of times, in any
27 +order. The commit log message will be composed in the order in which
28 +the options are given.
29 +
30 A commit object may have any number of parents. With exactly one
31 parent, it is an ordinary commit. Having more than one parent makes
32 the commit a merge between several lines of history. Initial (root)
@@ -41,7 +45,7 @@ state was.
45 OPTIONS
46 -------
47 <tree>::
44 - An existing tree object
48 + An existing tree object.
49
50 -p <parent>::
51 Each `-p` indicates the id of a parent commit object.
@@ -52,7 +56,8 @@ OPTIONS
56
57 -F <file>::
58 Read the commit log message from the given file. Use `-` to read
55 - from the standard input.
59 + from the standard input. This can be given more than once and the
60 + content of each file becomes its own paragraph.
61
62 -S[<keyid>]::
63 --gpg-sign[=<keyid>]::
builtin/commit-tree.c
+86 -72
@@ -12,8 +12,13 @@
12 #include "builtin.h"
13 #include "utf8.h"
14 #include "gpg-interface.h"
15 +#include "parse-options.h"
16
16 -static const char commit_tree_usage[] = "git commit-tree [(-p <sha1>)...] [-S[<keyid>]] [-m <message>] [-F <file>] <sha1>";
17 +static const char * const commit_tree_usage[] = {
18 + N_("git commit-tree [(-p <parent>)...] [-S[<keyid>]] [(-m <message>)...] "
19 + "[(-F <file>)...] <tree>"),
20 + NULL
21 +};
22
23 static const char *sign_commit;
24
@@ -23,7 +28,7 @@ static void new_parent(struct commit *parent, struct commit_list **parents_p)
28 struct commit_list *parents;
29 for (parents = *parents_p; parents; parents = parents->next) {
30 if (parents->item == parent) {
26 - error("duplicate parent %s ignored", oid_to_hex(oid));
31 + error(_("duplicate parent %s ignored"), oid_to_hex(oid));
32 return;
33 }
34 parents_p = &parents->next;
@@ -39,91 +44,100 @@ static int commit_tree_config(const char *var, const char *value, void *cb)
44 return git_default_config(var, value, cb);
45 }
46
47 +static int parse_parent_arg_callback(const struct option *opt,
48 + const char *arg, int unset)
49 +{
50 + struct object_id oid;
51 + struct commit_list **parents = opt->value;
52 +
53 + BUG_ON_OPT_NEG_NOARG(unset, arg);
54 +
55 + if (get_oid_commit(arg, &oid))
56 + die(_("not a valid object name %s"), arg);
57 +
58 + assert_oid_type(&oid, OBJ_COMMIT);
59 + new_parent(lookup_commit(the_repository, &oid), parents);
60 + return 0;
61 +}
62 +
63 +static int parse_message_arg_callback(const struct option *opt,
64 + const char *arg, int unset)
65 +{
66 + struct strbuf *buf = opt->value;
67 +
68 + BUG_ON_OPT_NEG_NOARG(unset, arg);
69 +
70 + if (buf->len)
71 + strbuf_addch(buf, '\n');
72 + strbuf_addstr(buf, arg);
73 + strbuf_complete_line(buf);
74 +
75 + return 0;
76 +}
77 +
78 +static int parse_file_arg_callback(const struct option *opt,
79 + const char *arg, int unset)
80 +{
81 + int fd;
82 + struct strbuf *buf = opt->value;
83 +
84 + BUG_ON_OPT_NEG_NOARG(unset, arg);
85 +
86 + if (buf->len)
87 + strbuf_addch(buf, '\n');
88 + if (!strcmp(arg, "-"))
89 + fd = 0;
90 + else {
91 + fd = open(arg, O_RDONLY);
92 + if (fd < 0)
93 + die_errno(_("git commit-tree: failed to open '%s'"), arg);
94 + }
95 + if (strbuf_read(buf, fd, 0) < 0)
96 + die_errno(_("git commit-tree: failed to read '%s'"), arg);
97 + if (fd && close(fd))
98 + die_errno(_("git commit-tree: failed to close '%s'"), arg);
99 +
100 + return 0;
101 +}
102 +
103 int cmd_commit_tree(int argc, const char **argv, const char *prefix)
104 {
44 - int i, got_tree = 0;
105 + static struct strbuf buffer = STRBUF_INIT;
106 struct commit_list *parents = NULL;
107 struct object_id tree_oid;
108 struct object_id commit_oid;
48 - struct strbuf buffer = STRBUF_INIT;
109 +
110 + struct option options[] = {
111 + { OPTION_CALLBACK, 'p', NULL, &parents, N_("parent"),
112 + N_("id of a parent commit object"), PARSE_OPT_NONEG,
113 + parse_parent_arg_callback },
114 + { OPTION_CALLBACK, 'm', NULL, &buffer, N_("message"),
115 + N_("commit message"), PARSE_OPT_NONEG,
116 + parse_message_arg_callback },
117 + { OPTION_CALLBACK, 'F', NULL, &buffer, N_("file"),
118 + N_("read commit log message from file"), PARSE_OPT_NONEG,
119 + parse_file_arg_callback },
120 + { OPTION_STRING, 'S', "gpg-sign", &sign_commit, N_("key-id"),
121 + N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
122 + OPT_END()
123 + };
124
125 git_config(commit_tree_config, NULL);
126
127 if (argc < 2 || !strcmp(argv[1], "-h"))
53 - usage(commit_tree_usage);
54 -
55 - for (i = 1; i < argc; i++) {
56 - const char *arg = argv[i];
57 - if (!strcmp(arg, "-p")) {
58 - struct object_id oid;
59 - if (argc <= ++i)
60 - usage(commit_tree_usage);
61 - if (get_oid_commit(argv[i], &oid))
62 - die("Not a valid object name %s", argv[i]);
63 - assert_oid_type(&oid, OBJ_COMMIT);
64 - new_parent(lookup_commit(the_repository, &oid),
65 - &parents);
66 - continue;
67 - }
128 + usage_with_options(commit_tree_usage, options);
129
69 - if (!strcmp(arg, "--gpg-sign")) {
70 - sign_commit = "";
71 - continue;
72 - }
130 + argc = parse_options(argc, argv, prefix, options, commit_tree_usage, 0);
131
74 - if (skip_prefix(arg, "-S", &sign_commit) ||
75 - skip_prefix(arg, "--gpg-sign=", &sign_commit))
76 - continue;
132 + if (argc != 1)
133 + die(_("must give exactly one tree"));
134
78 - if (!strcmp(arg, "--no-gpg-sign")) {
79 - sign_commit = NULL;
80 - continue;
81 - }
82 -
83 - if (!strcmp(arg, "-m")) {
84 - if (argc <= ++i)
85 - usage(commit_tree_usage);
86 - if (buffer.len)
87 - strbuf_addch(&buffer, '\n');
88 - strbuf_addstr(&buffer, argv[i]);
89 - strbuf_complete_line(&buffer);
90 - continue;
91 - }
92 -
93 - if (!strcmp(arg, "-F")) {
94 - int fd;
95 -
96 - if (argc <= ++i)
97 - usage(commit_tree_usage);
98 - if (buffer.len)
99 - strbuf_addch(&buffer, '\n');
100 - if (!strcmp(argv[i], "-"))
101 - fd = 0;
102 - else {
103 - fd = open(argv[i], O_RDONLY);
104 - if (fd < 0)
105 - die_errno("git commit-tree: failed to open '%s'",
106 - argv[i]);
107 - }
108 - if (strbuf_read(&buffer, fd, 0) < 0)
109 - die_errno("git commit-tree: failed to read '%s'",
110 - argv[i]);
111 - if (fd && close(fd))
112 - die_errno("git commit-tree: failed to close '%s'",
113 - argv[i]);
114 - continue;
115 - }
116 -
117 - if (get_oid_tree(arg, &tree_oid))
118 - die("Not a valid object name %s", arg);
119 - if (got_tree)
120 - die("Cannot give more than one trees");
121 - got_tree = 1;
122 - }
135 + if (get_oid_tree(argv[0], &tree_oid))
136 + die(_("not a valid object name %s"), argv[0]);
137
138 if (!buffer.len) {
139 if (strbuf_read(&buffer, 0, 0) < 0)
126 - die_errno("git commit-tree: failed to read");
140 + die_errno(_("git commit-tree: failed to read"));
141 }
142
143 if (commit_tree(buffer.buf, buffer.len, &tree_oid, parents, &commit_oid,
parse-options.h
+11
@@ -202,6 +202,17 @@ const char *optname(const struct option *opt, int flags);
202 BUG("option callback does not expect an argument"); \
203 } while (0)
204
205 +/*
206 + * Similar to the assertions above, but checks that "arg" is always non-NULL.
207 + * This assertion also implies BUG_ON_OPT_NEG(), letting you declare both
208 + * assertions in a single line.
209 + */
210 +#define BUG_ON_OPT_NEG_NOARG(unset, arg) do { \
211 + BUG_ON_OPT_NEG(unset); \
212 + if(!(arg)) \
213 + BUG("option callback expects an argument"); \
214 +} while(0)
215 +
216 /*----- incremental advanced APIs -----*/
217
218 enum {