From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by arlo.cworth.org (Postfix) with ESMTP id 45D866DE1EEF for ; Sun, 26 Feb 2017 11:36:00 -0800 (PST) X-Virus-Scanned: Debian amavisd-new at cworth.org X-Spam-Flag: NO X-Spam-Score: -0.17 X-Spam-Level: X-Spam-Status: No, score=-0.17 tagged_above=-999 required=5 tests=[AWL=-0.150, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01] autolearn=disabled Received: from arlo.cworth.org ([127.0.0.1]) by localhost (arlo.cworth.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id aaFSZ8sNYPBM for ; Sun, 26 Feb 2017 11:35:59 -0800 (PST) Received: from mail-lf0-f66.google.com (mail-lf0-f66.google.com [209.85.215.66]) by arlo.cworth.org (Postfix) with ESMTPS id 246096DE1E6C for ; Sun, 26 Feb 2017 11:35:59 -0800 (PST) Received: by mail-lf0-f66.google.com with SMTP id a198so2155953lfb.1 for ; Sun, 26 Feb 2017 11:35:59 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nikula-org.20150623.gappssmtp.com; s=20150623; h=from:to:subject:in-reply-to:references:date:message-id:mime-version; bh=a9Nn+xPfs6lQ4wF98kAk3fMKY2KBAjdxD9rhpJ2t+iE=; b=H0UIpz8xqD8rP23TTIa5tLMJult55yM7zCenkXpIpzENyAD3vWgieMCVw5yClAGSvD MrqUfoCo/+Mrg740EJUTzggjQKKkX1UTa+24j7RlsGzQobD5MMRzXi4jOMOxD644/hwJ Q0IKxjXNQI7xVpKqZYDH1oZdRWRAzxUgw1onjr9s4QIgCFPxVZWGMcb7oh20CYxO6SIu gOM0k/bvaTy7Q2EcjVvprBlhVXY0Z1/Jo7utT/wDTiDaRGVEqt8k+PFiipe73n+0llXe 0FU+OcweCiBlDErUaT3ooH+XlCXmzAGR9ypfBjiZW/ZUkv7k2skh5WnGyq2qDRmuodwK 0zWQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:in-reply-to:references:date :message-id:mime-version; bh=a9Nn+xPfs6lQ4wF98kAk3fMKY2KBAjdxD9rhpJ2t+iE=; b=lAEPlV4tADGSAGGbwch3C7JGTn2FkF9DLE5FJbezFMCWtP8AbZa6Vzk7OEf2vGDuXR YbtOWaGqAEKUs7Ck92MQaC56B8VttQFNpb2FJZrw+NPk2SX6jIgZO+4FzHZJ+cs4w3vq kNrd97TWvloKEide9gMojgU7ZWNzJRmGgfx7L0E6CheW+nXitpXdOHFHHAZtVoaIx4QN aPzEtdataz6zKIucX14fY2+yuaGoasSZMJmYvtOCcYVQic7qQ/JG3pwqfneTqAENXMqf Sl8+uhD7bzvntp1IWcAU1vczlou7nyIDFufRgAHqm+opQ9yIAXY9F6RrN4yz1lLTiZqx jXeA== X-Gm-Message-State: AMke39lKJMtoP35m+8pzFI8qhWVfx6IQ75IUcOAZA9WKPUSnslNntnHcD+i/F1GlNUD94w== X-Received: by 10.46.72.2 with SMTP id v2mr3185136lja.5.1488137757519; Sun, 26 Feb 2017 11:35:57 -0800 (PST) Received: from localhost (mobile-access-bcee80-14.dhcp.inet.fi. [188.238.128.14]) by smtp.gmail.com with ESMTPSA id q71sm19732lfe.40.2017.02.26.11.35.56 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Sun, 26 Feb 2017 11:35:56 -0800 (PST) From: Jani Nikula To: David Bremner , notmuch@notmuchmail.org Subject: Re: [patch v5 3/6] lib: create field processors from prefix table In-Reply-To: <20170217030754.32069-4-david@tethera.net> References: <20170217030754.32069-1-david@tethera.net> <20170217030754.32069-4-david@tethera.net> Date: Sun, 26 Feb 2017 21:35:54 +0200 Message-ID: <87innwhhid.fsf@nikula.org> MIME-Version: 1.0 Content-Type: text/plain X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 26 Feb 2017 19:36:00 -0000 On Thu, 16 Feb 2017, David Bremner 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