From: Jani Nikula <jani@nikula.org>
To: David Bremner <david@tethera.net>, notmuch@notmuchmail.org
Subject: Re: [patch v5 3/6] lib: create field processors from prefix table
Date: Sun, 26 Feb 2017 21:35:54 +0200 [thread overview]
Message-ID: <87innwhhid.fsf@nikula.org> (raw)
In-Reply-To: <20170217030754.32069-4-david@tethera.net>
On Thu, 16 Feb 2017, David Bremner <david@tethera.net> wrote:
> This is a bit more code than hardcoding the two existing field
> processors, but it should make it easy to add more.
> ---
> lib/database-private.h | 3 ++-
> lib/database.cc | 45 +++++++++++++++++++++++++++++++--------------
> 2 files changed, 33 insertions(+), 15 deletions(-)
>
> diff --git a/lib/database-private.h b/lib/database-private.h
> index 2fb60f5e..60d1fead 100644
> --- a/lib/database-private.h
> +++ b/lib/database-private.h
> @@ -153,7 +153,8 @@ operator&=(_notmuch_features &a, _notmuch_features b)
> typedef enum notmuch_field_flags {
> NOTMUCH_FIELD_NO_FLAGS = 0,
> NOTMUCH_FIELD_EXTERNAL = 1 << 0,
> - NOTMUCH_FIELD_PROBABILISTIC = 1 << 1
> + NOTMUCH_FIELD_PROBABILISTIC = 1 << 1,
> + NOTMUCH_FIELD_PROCESSOR = 1 << 2
Nitpick, if you add a comma at the end, subsequent changes will have
smaller diff.
> } notmuch_field_flag_t;
>
> /*
> diff --git a/lib/database.cc b/lib/database.cc
> index 8016c4df..450ee295 100644
> --- a/lib/database.cc
> +++ b/lib/database.cc
> @@ -270,6 +270,12 @@ prefix_t prefix_table[] = {
> * discussion.
> */
> { "folder", "XFOLDER:", NOTMUCH_FIELD_EXTERNAL },
> +#if HAVE_XAPIAN_FIELD_PROCESSOR
> + { "date", NULL, NOTMUCH_FIELD_EXTERNAL |
> + NOTMUCH_FIELD_PROCESSOR },
> + { "query", NULL, NOTMUCH_FIELD_EXTERNAL |
> + NOTMUCH_FIELD_PROCESSOR },
> +#endif
> { "from", "XFROM", NOTMUCH_FIELD_EXTERNAL |
> NOTMUCH_FIELD_PROBABILISTIC },
> { "to", "XTO", NOTMUCH_FIELD_EXTERNAL |
> @@ -282,6 +288,20 @@ prefix_t prefix_table[] = {
> NOTMUCH_FIELD_PROBABILISTIC },
> };
>
> +#if HAVE_XAPIAN_FIELD_PROCESSOR
> +static Xapian::FieldProcessor *
> +_make_field_processor (const char *name, notmuch_database_t *notmuch) {
> + if (STRNCMP_LITERAL (name, "date") == 0)
> + return (new DateFieldProcessor())->release ();
> + else if (STRNCMP_LITERAL(name, "query") == 0)
> + return (new QueryFieldProcessor (*notmuch->query_parser, notmuch))->release ();
> +
> + INTERNAL_ERROR ("no field processor for prefix '%s'\n", name);
> +}
> +#else
> +#define _make_field_processor(name, db) NULL
Nitpick, if you make this a proper static inline function that just
returns NULL, you'll get type checking but the end result will be the
same.
> +#endif
> +
> const char *
> _find_prefix (const char *name)
> {
> @@ -1027,18 +1047,6 @@ notmuch_database_open_verbose (const char *path,
> notmuch->term_gen->set_stemmer (Xapian::Stem ("english"));
> notmuch->value_range_processor = new Xapian::NumberValueRangeProcessor (NOTMUCH_VALUE_TIMESTAMP);
> notmuch->date_range_processor = new ParseTimeValueRangeProcessor (NOTMUCH_VALUE_TIMESTAMP);
> -#if HAVE_XAPIAN_FIELD_PROCESSOR
> - /* This currently relies on the query parser to pass anything
> - * with a .. to the range processor */
> - {
> - Xapian::FieldProcessor * date_fp = new DateFieldProcessor();
> - Xapian::FieldProcessor * query_fp =
> - new QueryFieldProcessor (*notmuch->query_parser, notmuch);
> -
> - notmuch->query_parser->add_boolean_prefix("date", date_fp->release ());
> - notmuch->query_parser->add_boolean_prefix("query", query_fp->release ());
> - }
> -#endif
> notmuch->last_mod_range_processor = new Xapian::NumberValueRangeProcessor (NOTMUCH_VALUE_LAST_MOD, "lastmod:");
>
> notmuch->query_parser->set_default_op (Xapian::Query::OP_AND);
> @@ -1052,8 +1060,17 @@ notmuch_database_open_verbose (const char *path,
> for (i = 0; i < ARRAY_SIZE (prefix_table); i++) {
> const prefix_t *prefix = &prefix_table[i];
> if (prefix->flags & NOTMUCH_FIELD_EXTERNAL) {
> - if (prefix->flags & NOTMUCH_FIELD_PROBABILISTIC) {
> - notmuch->query_parser->add_prefix (prefix->name, prefix->prefix);
> + /* we treat all field-processor fields as boolean in order
> + to get the raw input */
> + if (HAVE_XAPIAN_FIELD_PROCESSOR &&
> + (prefix->flags & NOTMUCH_FIELD_PROCESSOR)) {
> + Xapian::FieldProcessor *fp = _make_field_processor (prefix->name,
> + notmuch);
> +
> + notmuch->query_parser->add_boolean_prefix (prefix->name, fp);
Are you sure this builds for HAVE_XAPIAN_FIELD_PROCESSOR=0? I think this
assumes the compiler eliminates the dead code before actually compiling
it. Xapian::FieldProcessor is not there for older Xapian, is it?
I strongly prefer having conditionally compilation at the function level
(like _make_field_processor) but I think you'll need to move everything
from this if block in that function. I admit it's not the prettiest
thing when the other ->add_prefix calls are at this level.
> + } else if (prefix->flags & NOTMUCH_FIELD_PROBABILISTIC) {
> + notmuch->query_parser->add_prefix (prefix->name,
> + prefix->prefix);
> } else {
> notmuch->query_parser->add_boolean_prefix (prefix->name,
> prefix->prefix);
> --
> 2.11.0
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch
next prev parent reply other threads:[~2017-02-26 19:36 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-17 3:07 v5 of regexp searching David Bremner
2017-02-17 3:07 ` [patch v5 1/6] lib: merge internal prefix tables David Bremner
2017-02-17 3:07 ` [patch v5 2/6] lib: Let Xapian manage the memory for FieldProcessors David Bremner
2017-02-19 21:43 ` David Bremner
2017-02-17 3:07 ` [patch v5 3/6] lib: create field processors from prefix table David Bremner
2017-02-26 19:35 ` Jani Nikula [this message]
2017-02-17 3:07 ` [patch v5 4/6] lib: regexp matching in 'subject' and 'from' David Bremner
2017-02-26 19:46 ` Jani Nikula
2017-02-17 3:07 ` [patch v5 5/6] lib: add mid: as a synonym for id: David Bremner
2017-02-26 19:52 ` Jani Nikula
2017-02-17 3:07 ` [patch v5 6/6] lib: Add regexp searching for mid: prefix David Bremner
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=87innwhhid.fsf@nikula.org \
--to=jani@nikula.org \
--cc=david@tethera.net \
--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).