Re: [PATCHv13 28/30] builtin-notes: Add -c/-C options for reusing notes

[Johan Herland <johan@herland.net>]

On Monday 15 February 2010, Stephen Boyd wrote:
> On 02/13/2010 01:28 PM, Johan Herland wrote:
> > @@ -199,6 +203,40 @@ static int parse_file_arg(const struct option
> > *opt, const char *arg, int unset)
> > 
> >  	return 0;
> >  
> >  }
> > 
> > +static int parse_reuse_arg(const struct option *opt, const char *arg,
> > int unset) +{
> > +	struct msg_arg *msg = opt->value;
> > +	char *buf;
> > +	unsigned char object[20];
> > +	enum object_type type;
> > +	unsigned long len;
> > +
> > +	if (!arg)
> > +		return -1;
> 
> This is impossible unless you're using the PARSE_OPT_OPTARG flag or
> allowing negation (i.e. --no-reuse-mesage). You should probably make the
> two callback options PARSE_OPT_NONEG and then drop this if statement.
> Same applies to some of the other callbacks not introduced in this patch.

Thanks. Fixed locally. Will be part of the next iteration.

> > +
> > +	if (msg->buf.len)
> > +		strbuf_addstr(&(msg->buf), "\n");
> > +
> 
> Use strbuf_addch()? I saw this in a couple other patches too.

Of course. Thanks for noticing. Fixed locally. Will be part of the next 
iteration.


Thanks for the review! :)

...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net