[PATCH v3 2/9] parse-time-string: add a date/time parser to notmuch

Jani Nikula jani at nikula.org
Thu Sep 13 05:07:35 PDT 2012


On Thu, 13 Sep 2012, Michal Nazarewicz <mina86 at mina86.com> wrote:
> On Wed, Sep 12 2012, Jani Nikula <jani at nikula.org> wrote:
>> Add a date/time parser to notmuch, to be used for adding date range
>> query support for notmuch lib later on. Add the parser to a directory
>> of its own to make it independent of the rest of the notmuch code
>> base.
>>
>> Signed-off-by: Jani Nikula <jani at nikula.org>
>
> Have you consider doing the same in bison?  I consider the code totally
> unreadable and unmaintainable.

I do not think you could easily do everything that this parser does in
bison. But then I'm not an expert in bison, and I have zero ambition to
become one. So I'm biased, and I'm open about it.

Even so, if you're suggesting doing this in bison would make this
totally readable and maintainable, I urge you to have a good look at
[1]. Note that it also does less in more lines of code. (And using it
as-is in notmuch has pretty much been turned down in the past.)

Finally, I also suggest you actually read and review the code, pointing
out concrete issues in readability or maintainability that you
see. Especially since an earlier version has received comment "[I]t
looks very nice to me. It is pleasantly nice to read." [2]. What you're
doing is worthless bikeshedding otherwise.

BR,
Jani.

[1] http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob_plain;f=lib/parse-datetime.y
[2] id:"87mx86dlul.fsf at qmul.ac.uk"


More information about the notmuch mailing list