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

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

From
Andrei Rybak <rybak.a.v@gmail.com>
Date
Feb 26, 2019, 22:38 UTC
Message-ID
<33efa988-ea80-d9b4-f4aa-3876331a1dfb@gmail.com>
In-Reply-To
<20190226200952.33950-1-brandon1024.br@gmail.com>
A couple of code style issues:
On 2/26/19 9:09 PM, Brandon wrote:
Show 9 quoted lines
> 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
Missing space after period.
[snip]
Show 11 quoted lines
> +
>  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[] = {
Style: tab should be used instead of four spaces.
> +		{ OPTION_CALLBACK, 'p', NULL, &parents, "parent",
> +		  N_("id of a parent commit object"), PARSE_OPT_NONEG,

Comparing to other similar places, a single tab should be used to align "N_" instead of two spaces.

Show 11 quoted lines
> +		  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()
> +    };
[snip]
Show 7 quoted lines
> -
> -		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);

here "builtin_commit_tree_usage" should be aligned with "argc" in previous line.

Previous: BrandonNext: Brandon Richardson
Message 2 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.