From: Austin Clements <amdragon@MIT.EDU>
To: Jani Nikula <jani@nikula.org>
Cc: notmuch@notmuchmail.org
Subject: Re: [PATCH v5 2/9] parse-time-string: add a date/time parser to notmuch
Date: Thu, 25 Oct 2012 14:58:16 -0400 [thread overview]
Message-ID: <20121025185816.GX14861@mit.edu> (raw)
In-Reply-To: <20121022081444.GM14861@mit.edu>
Quoth myself on Oct 22 at 4:14 am:
> Overall this looks pretty good to me, and I must say, this parser is
> amazingly flexible and copes well with a remarkably hostile grammar.
>
> A lot of little comments below (sorry if any of this ground has
> already been covered in the previous four versions).
>
> I do have one broad comment. While I'm all for ad hoc parsers for ad
> hoc grammars like dates, there is one piece of the literature I think
> this parser suffers for by ignoring: tokenizing. I think it would
> simplify a lot of this code if it did a tokenizing pass before the
> parsing pass. It doesn't have to be a serious tokenizer with
> streaming and keywords and token types and junk; just something that
> first splits the input into substrings, possibly just non-overlapping
> matches of [[:digit:]]+|[[:alpha:]]+|[-+:/.]. This would simplify the
> handling of postponed numbers because, with trivial lookahead in the
> token stream, you wouldn't have to postpone them. Likewise, it would
> eliminate last_field. It would simplify keyword matching because you
> wouldn't have to worry about matching substrings (I spent a long time
> staring at that code before I figured out what it would and wouldn't
> accept). Most important, I think it would make the parser more
> predictable for users; for example, the parser currently accepts
> things like "saturtoday" because it's aggressively single-pass.
I should add that I am not at all opposed to this patch as it is
currently designed. We need a date parser. My comment about
separating tokenization is just a way that this code could probably be
simplified if someone were so inclined or if simplifying the code
would help it pass any hurdles.
next prev parent reply other threads:[~2012-10-25 18:58 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-10-21 21:22 [PATCH v5 0/9] notmuch search date:since..until query support Jani Nikula
2012-10-21 21:22 ` [PATCH v5 1/9] build: drop the -Wswitch-enum warning Jani Nikula
2012-10-21 21:22 ` [PATCH v5 2/9] parse-time-string: add a date/time parser to notmuch Jani Nikula
2012-10-22 8:14 ` Austin Clements
2012-10-25 18:58 ` Austin Clements [this message]
2012-10-27 20:38 ` Tomi Ollila
2012-10-28 22:30 ` Jani Nikula
2012-10-28 22:52 ` Austin Clements
2012-10-21 21:22 ` [PATCH v5 3/9] test: add new test tool parse-time for date/time parser Jani Nikula
2012-10-21 21:22 ` [PATCH v5 4/9] test: add smoke tests for the date/time parser module Jani Nikula
2012-10-23 4:23 ` Austin Clements
2012-10-28 22:34 ` Jani Nikula
2012-10-21 21:22 ` [PATCH v5 5/9] build: build parse-time-string as part of the notmuch lib and static cli Jani Nikula
2012-10-21 21:22 ` [PATCH v5 6/9] lib: add date range query support Jani Nikula
2012-10-23 4:52 ` Austin Clements
2012-10-28 22:39 ` Jani Nikula
2012-10-21 21:22 ` [PATCH v5 7/9] test: add tests for date:since..until range queries Jani Nikula
2012-10-21 21:22 ` [PATCH v5 8/9] man: document the " Jani Nikula
2012-10-24 21:08 ` Austin Clements
2012-10-28 22:41 ` Jani Nikula
2012-10-21 21:22 ` [PATCH v5 9/9] NEWS: date range search support Jani Nikula
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://notmuchmail.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20121025185816.GX14861@mit.edu \
--to=amdragon@mit.edu \
--cc=jani@nikula.org \
--cc=notmuch@notmuchmail.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://yhetil.org/notmuch.git/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).