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

2019-02-28 Thread Jeff King
On Wed, Feb 27, 2019 at 10:46:49PM -0400, Brandon Richardson wrote: > > If we are going to go this route, I think you might actually want macros > > that take both "unset" and "args" and make sure that we're not in a > > situation the callback doesn't expect (e.g., "!unset && !arg"). That > > lets

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

2019-02-27 Thread Duy Nguyen
On Wed, Feb 27, 2019 at 10:24 PM Brandon Richardson wrote: > > > +static int parse_file_arg_callback(const struct option *opt, > > > + const char *arg, int unset) > > > > I would suggest you do the same for -F, i.e. collect a string list of > > paths then do the heavy lifting afterwa

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

2019-02-27 Thread Duy Nguyen
On Wed, Feb 27, 2019 at 7:36 PM SZEDER Gábor wrote: > > On Wed, Feb 27, 2019 at 06:49:53PM +0700, Duy Nguyen wrote: > > On Wed, Feb 27, 2019 at 6:37 PM SZEDER Gábor wrote: > > > > > > On Wed, Feb 27, 2019 at 06:07:42PM +0700, Duy Nguyen wrote: > > > > > It was discovered that the --no-gpg-sign op

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

2019-02-27 Thread Brandon Richardson
Hi Jeff, > One of the reasons I did not bother with that condition when I added the > OPT_NEG() and OPT_ARG() variants is that you can only get an unexpected > NULL argument if you explicitly give the NOARG or OPTARG flags. So it's > very easy to _forget_ to give such a flag, because you simply ar

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

2019-02-27 Thread Jeff King
On Wed, Feb 27, 2019 at 06:07:42PM +0700, Duy Nguyen wrote: > > +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_O

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

2019-02-27 Thread Brandon Richardson
Thank you all for the helpful comments :-) On Wed, 27 Feb 2019 at 07:08, Duy Nguyen wrote: > > It was discovered that the --no-gpg-sign option was documented > > but not implemented in 55ca3f99, and the existing implementation > > Most people refer to a commit with this format > > 55ca3f99ae (com

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

2019-02-27 Thread SZEDER Gábor
On Wed, Feb 27, 2019 at 06:49:53PM +0700, Duy Nguyen wrote: > On Wed, Feb 27, 2019 at 6:37 PM SZEDER Gábor wrote: > > > > On Wed, Feb 27, 2019 at 06:07:42PM +0700, Duy Nguyen wrote: > > > > It was discovered that the --no-gpg-sign option was documented > > > > but not implemented in 55ca3f99, and

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

2019-02-27 Thread Duy Nguyen
On Wed, Feb 27, 2019 at 6:37 PM SZEDER Gábor wrote: > > On Wed, Feb 27, 2019 at 06:07:42PM +0700, Duy Nguyen wrote: > > > It was discovered that the --no-gpg-sign option was documented > > > but not implemented in 55ca3f99, and the existing implementation > > > > Most people refer to a commit with

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

2019-02-27 Thread SZEDER Gábor
On Wed, Feb 27, 2019 at 06:07:42PM +0700, Duy Nguyen wrote: > > It was discovered that the --no-gpg-sign option was documented > > but not implemented in 55ca3f99, and the existing implementation > > Most people refer to a commit with this format > > 55ca3f99ae (commit-tree: add and document --no

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

2019-02-27 Thread Duy Nguyen
On Wed, Feb 27, 2019 at 6:43 AM Brandon Richardson wrote: > > Hi Andrei, > > > > would attempt to translate the option as a tree oid.It was also > > > > Missing space after period. > > Oops, thanks for pointing that out. > > > > > > + { OPTION_CALLBACK, 'p', NULL, &parents, "parent", >

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

2019-02-27 Thread Duy Nguyen
On Wed, Feb 27, 2019 at 3:10 AM Brandon wrote: > > From: Brandon Richardson > > 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 docum

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

2019-02-26 Thread Brandon Richardson
Hi Andrei, > > would attempt to translate the option as a tree oid.It was also > > Missing space after period. Oops, thanks for pointing that out. > > > + { OPTION_CALLBACK, 'p', NULL, &parents, "parent", > > + N_("id of a parent commit object"), PARSE_OPT_NONEG, > > Co

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

2019-02-26 Thread Andrei Rybak
A couple of code style issues: On 2/26/19 9:09 PM, Brandon wrote: > From: Brandon Richardson > > 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-si

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

2019-02-26 Thread Brandon
From: Brandon Richardson 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 implem