From: Tomi Ollila <tomi.ollila@iki.fi>
To: David Bremner <david@tethera.net>, notmuch@notmuchmail.org
Subject: Re: [Patch v2 04/13] lib/cli: add library API / CLI for compile time
 options
In-Reply-To: <1459015043-8460-5-git-send-email-david@tethera.net>
References: <1459015043-8460-1-git-send-email-david@tethera.net>
 <1459015043-8460-5-git-send-email-david@tethera.net>
User-Agent: Notmuch/0.22+7~g2ab49fe (http://notmuchmail.org) Emacs/24.3.1
 (x86_64-unknown-linux-gnu)
Date: Wed, 27 Apr 2016 20:47:04 +0300
Message-ID: <m27ffjvt9j.fsf@guru.guru-group.fi>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Precedence: list

On Sat, Mar 26 2016, David Bremner <david@tethera.net> wrote:

> This is intentionally low tech; if we have more than two options it may
> make sense to build up what infrastructure is provided.

2 quick notes for this patch -- since it has been like a month when I
looked this it takes time to get re-acquainted to the series again

...IMO this compile time "options" feels a bit odd; what options those are
if those are not options (but fixed things on what is available on the
underlying libraries). something like "features" would be a little less
odd (if there were no "database features" there ;/)

another thing is that these fixed things that were resolved by compile time=
=20
is (re-)checked in compile time. instead of that just the=20
HAVE_XAPIAN_COMPACT and HAVE_XAPIAN_FIELD_PROCESSOR could be used

... like:=20

notmuch_bool_t
notmuch_options_get (const char *name) {
    if (STRNCMP_LITERAL (name, "compact") =3D=3D 0) {
	return HAVE_XAPIAN_COMPACT;
	// or return HAVE_XAPIAN_COMPACT? TRUE: FALSE;
	// or return !! HAVE_XAPIAN_COMPACT;
    } else if (STRNCMP_LITERAL (name, "field_processor") =3D=3D 0) {
	return HAVE_XAPIAN_FIELD_PROCESSOR;
    } else {
	return FALSE;
    }
}

... note: if this notmuch_options_present () function prevails, fix
the later #if check ;D

... and

void
_notmuch_config_list_options () {
    printf("options.compact=3D%s\n",
	   HAVE_XAPIAN_COMPACT ? "true" : "false");
    printf("options.field_processor=3D%s\n",
	   HAVE_XAPIAN_FIELD_PROCESSOR ? "true" : "false");
}

(kept the 'options' naming and prefix in this context)

otoh, all this "options" reading using notmuch config interface could
be pos^H^H^H^H put into another series and this naming thing could be
resolved there (proviced that these #if HAVE_XAPIAN ... #endif constructs
can be used conveniently here)

this is how these things look to me now; as usual i may not have=20
caught all of the considerations here.

Tomi


> ---
>  doc/man1/notmuch-config.rst |  5 +++++
>  lib/Makefile.local          |  1 +
>  lib/notmuch.h               | 10 +++++++++
>  lib/options.c               | 50 +++++++++++++++++++++++++++++++++++++++=
++++++
>  notmuch-config.c            | 20 ++++++++++++++++++
>  test/T030-config.sh         |  6 ++++--
>  test/T040-setup.sh          |  6 ++++--
>  test/test-lib.sh            |  6 ++++++
>  8 files changed, 100 insertions(+), 4 deletions(-)
>  create mode 100644 lib/options.c
>
> diff --git a/doc/man1/notmuch-config.rst b/doc/man1/notmuch-config.rst
> index 40c1272..150d764 100644
> --- a/doc/man1/notmuch-config.rst
> +++ b/doc/man1/notmuch-config.rst
> @@ -132,6 +132,11 @@ The available configuration items are described belo=
w.
>=20=20=20=20=20=20
>          Default: ``gpg``.
>=20=20
> +    **options.<name>**
> +
> +	Compile time option <name>. Current possibilities include
> +	"compact" (see **notmuch-compact(1)**)
> +	and "field_processor" (see **notmuch-search-terms(7)**).
>=20=20
>  ENVIRONMENT
>  =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> diff --git a/lib/Makefile.local b/lib/Makefile.local
> index 3a07090..4ad0158 100644
> --- a/lib/Makefile.local
> +++ b/lib/Makefile.local
> @@ -39,6 +39,7 @@ libnotmuch_c_srcs =3D		\
>  	$(dir)/message-file.c	\
>  	$(dir)/messages.c	\
>  	$(dir)/sha1.c		\
> +	$(dir)/options.c	\
>  	$(dir)/tags.c
>=20=20
>  libnotmuch_cxx_srcs =3D		\
> diff --git a/lib/notmuch.h b/lib/notmuch.h
> index cb46fc0..b29dd5f 100644
> --- a/lib/notmuch.h
> +++ b/lib/notmuch.h
> @@ -1838,6 +1838,16 @@ notmuch_filenames_move_to_next (notmuch_filenames_=
t *filenames);
>  void
>  notmuch_filenames_destroy (notmuch_filenames_t *filenames);
>=20=20
> +typedef enum {
> +    NOTMUCH_OPTION_COMPACT =3D 1,
> +    NOTMUCH_OPTION_FIELD_PROCESSOR =3D 2
> +} notmuch_option_t;
> +
> +notmuch_bool_t
> +notmuch_options_present (notmuch_option_t mask);
> +
> +notmuch_bool_t
> +notmuch_options_get (const char *name);
>  /* @} */
>=20=20
>  NOTMUCH_END_DECLS
> diff --git a/lib/options.c b/lib/options.c
> new file mode 100644
> index 0000000..4e15d92
> --- /dev/null
> +++ b/lib/options.c
> @@ -0,0 +1,50 @@
> +/* notmuch - Not much of an email program, (just index and search)
> + *
> + * Copyright =C2=A9 2016 David Bremner
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 3 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see http://www.gnu.org/licenses/ .
> + *
> + * Author: David Bremner <david@tethera.net>
> + */
> +
> +#include "notmuch.h"
> +#include "notmuch-private.h"
> +
> +notmuch_bool_t
> +notmuch_options_present (notmuch_option_t mask)
> +{
> +    notmuch_option_t present =3D 0;
> +
> +#if HAVE_XAPIAN_COMPACT
> +    present |=3D NOTMUCH_OPTION_COMPACT;
> +#endif
> +
> +#if HAVE_XAPIAN_COMPACT
> +    present |=3D NOTMUCH_OPTION_FIELD_PROCESSOR;
> +#endif
> +
> +    return (mask & present) !=3D 0;
> +
> +}
> +
> +notmuch_bool_t
> +notmuch_options_get (const char *name) {
> +    if (STRNCMP_LITERAL (name, "compact") =3D=3D 0) {
> +	return notmuch_options_present (NOTMUCH_OPTION_COMPACT);
> +    } else if (STRNCMP_LITERAL (name, "field_processor") =3D=3D 0) {
> +	return notmuch_options_present (NOTMUCH_OPTION_FIELD_PROCESSOR);
> +    } else {
> +	return FALSE;
> +    }
> +}
> diff --git a/notmuch-config.c b/notmuch-config.c
> index d252bb2..cfc549d 100644
> --- a/notmuch-config.c
> +++ b/notmuch-config.c
> @@ -750,6 +750,8 @@ _item_split (char *item, char **group, char **key)
>      return 0;
>  }
>=20=20
> +#define OPTION_PREFIX "options."
> +
>  static int
>  notmuch_config_command_get (notmuch_config_t *config, char *item)
>  {
> @@ -773,6 +775,9 @@ notmuch_config_command_get (notmuch_config_t *config,=
 char *item)
>  	tags =3D notmuch_config_get_new_tags (config, &length);
>  	for (i =3D 0; i < length; i++)
>  	    printf ("%s\n", tags[i]);
> +    } else if (STRNCMP_LITERAL (item, OPTION_PREFIX) =3D=3D 0) {
> +	printf ("%s\n",
> +	       notmuch_options_get (item + strlen (OPTION_PREFIX)) ? "true" : "=
false");
>      } else {
>  	char **value;
>  	size_t i, length;
> @@ -804,6 +809,11 @@ notmuch_config_command_set (notmuch_config_t *config=
, char *item, int argc, char
>  {
>      char *group, *key;
>=20=20
> +    if (STRNCMP_LITERAL (item, OPTION_PREFIX) =3D=3D 0) {
> +	fprintf (stderr, "Error: read only option: %s\n", item);
> +	return 1;
> +    }
> +
>      if (_item_split (item, &group, &key))
>  	return 1;
>=20=20
> @@ -830,6 +840,15 @@ notmuch_config_command_set (notmuch_config_t *config=
, char *item, int argc, char
>      return notmuch_config_save (config);
>  }
>=20=20
> +static
> +void
> +_notmuch_config_list_options () {
> +    printf("options.compact=3D%s\n",
> +	   notmuch_options_present(NOTMUCH_OPTION_COMPACT) ? "true" : "false");
> +    printf("options.field_processor=3D%s\n",
> +	   notmuch_options_present(NOTMUCH_OPTION_FIELD_PROCESSOR) ? "true" : "=
false");
> +}