git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrei Rybak <rybak.a.v@gmail.com>
To: Brandon <brandon1024.br@gmail.com>, git@vger.kernel.org
Subject: Re: [PATCH] commit-tree: utilize parse-options api
Date: Tue, 26 Feb 2019 23:38:00 +0100	[thread overview]
Message-ID: <33efa988-ea80-d9b4-f4aa-3876331a1dfb@gmail.com> (raw)
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:
> 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]

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


  reply	other threads:[~2019-02-26 22:38 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-26 20:09 [PATCH] commit-tree: utilize parse-options api Brandon
2019-02-26 22:38 ` Andrei Rybak [this message]
2019-02-26 23:42   ` Brandon Richardson
2019-02-27 11:13     ` Duy Nguyen
2019-02-27 11:07 ` Duy Nguyen
2019-02-27 11:37   ` SZEDER Gábor
2019-02-27 11:49     ` Duy Nguyen
2019-02-27 12:36       ` SZEDER Gábor
2019-02-28  7:21         ` Duy Nguyen
2019-02-27 15:24   ` Brandon Richardson
2019-02-28  7:26     ` Duy Nguyen
2019-02-27 16:35   ` Jeff King
2019-02-28  2:46     ` Brandon Richardson
2019-02-28 20:56       ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=33efa988-ea80-d9b4-f4aa-3876331a1dfb@gmail.com \
    --to=rybak.a.v@gmail.com \
    --cc=brandon1024.br@gmail.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).