Re: [PATCH 2/4] notmuch: Config option to specify tags to be applied by 'notmuch new'.

[Jan Janak <jan@ryngle.com>]

Hi Bart,

On 25-11 15:56, Bart Trojanowski wrote:
> Jan,
> 
> I really want this feature to get in, so I am going to do my best to
> review your code :)
> 
> Here are some more sticking points...
> 
> > +char **
> > +notmuch_config_get_new_tags (notmuch_config_t *config, size_t *length);
> 
> If you are not giving over control of the pointer to the caller please
> return const char * const *.

I followed Carl's style there, in particular the following function:
notmuch_config_get_user_other_email

I can, of course, change that. But maybe we should wait for Carl to see
which way he prefers.

> Similarly...
> 
> > +    char **new_tags;
> 
> ... this should probably be const char **.

That's the same story. I followed user_other_email there.

> Next...
> 
> > +char **
> > +notmuch_config_get_new_tags (notmuch_config_t *config, size_t *length)
> 
> ... but ...
> 
> > +    unsigned int count, i;
> > +
> > +    if ((tags = notmuch_config_get_new_tags (config, &count)) == NULL)
> > +	return;
> 
> size_t != unsigned int on all platforms.  Please stick with one or the
> other.  Note there are a few calls to fix here.

That's a good catch. I will fix that one. Thanks a lot for the review, I
really appreciate that!

  -- Jan