Web lists-archives.com

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




A couple of code style issues:

On 2/26/19 9:09 PM, Brandon wrote:
> From: Brandon Richardson <brandon1024.br@xxxxxxxxx>
> 
> 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]

> +
>  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.

> +		  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]

> -
> -		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.