[patch v5 3/6] lib: create field processors from prefix table

[Jani Nikula]

On Thu, 16 Feb 2017, David Bremner <david at 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 at notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch