[PATCH 5/6] cli/new: support /<regex>/ in new.ignore

David Bremner david at tethera.net
Sun Oct 1 18:00:24 PDT 2017


Jani Nikula <jani at nikula.org> writes:
>>
>> One thing we eventually settled on in the query parser is that an
>> opening '/' without a trailing '/' is an errror. But perhaps it's fine
>> to take a more permissive approach here.
>
> I'm fine either way, I just chose to be permissive.
>
> So do I make the function void and drop the return values, or make it
> check and return errors?

I think I'd prefer to start strict, it's easier to become permissive later.

>
>>
>>> +
>>> +    if (! state->ignore_regex_length)
>>> +	return FALSE;
>>
>> It's a nitpick, even by the standards of this review, but I'd prefer an
>> explicit '> 0' check.
>
> ITYM (state->ignore_regex_length == 0) but ack.
>

yeah, thought of that after just after I sent it, but yes.


More information about the notmuch mailing list