From: Daniel Schoepe <daniel.schoepe@googlemail.com>
To: Michal Sojka <sojkam1@fel.cvut.cz>, notmuch@notmuchmail.org
Subject: Re: [PATCH v4 1/2] emacs: User-defined sections in notmuch-hello
In-Reply-To: <87box48jkc.fsf@steelpick.2x.cz>
References:
 <id:"1309379221-5617-1-git-send-email-daniel.schoepe@googlemail.com">
	<1310079227-19120-1-git-send-email-daniel.schoepe@googlemail.com>
	<1310079227-19120-2-git-send-email-daniel.schoepe@googlemail.com>
	<87box48jkc.fsf@steelpick.2x.cz>
User-Agent: Notmuch/0.5-329-g0b5d38d (http://notmuchmail.org) Emacs/23.3.1
	(x86_64-pc-linux-gnu)
Date: Sat, 09 Jul 2011 01:13:43 +0200
Message-ID: <87mxgo2wns.fsf@gilead.invalid>
MIME-Version: 1.0
Content-Type: multipart/signed; boundary="=-=-=";
	micalg=pgp-sha1; protocol="application/pgp-signature"
Precedence: list

--=-=-=
Content-Transfer-Encoding: quoted-printable

On Sat, 09 Jul 2011 01:00:03 +0200, Michal Sojka <sojkam1@fel.cvut.cz> wrot=
e:
> Yes, this is definitely a good idea. My last comment to the patch is
> that I do not like the use of plists in customization interface. It is
> especially weird in the case of boolean options like hide-if-empty,
> because they actually have three states: disabled, off and on. And this
> make not a lot of sense.
>=20
> I think that the customization interface is much better arranged when it
> is modified like in the patch below (it is against v3).
>=20
> [..]
> -  :type
> -  (let ((opts
> -        '((:title (string :tag "Title for this section"))
> -          (:make-query (string :tag "Filter for each tag"))
> -          (:make-count (string :tag "Different query to generate counts"=
))
> -          (:hide-tags (repeat :tag "Tags that will be hidden" string))
> -          (:initially-hidden (boolean :tag "Hide this on startup?"))
> -          (:hide-empty-tags (boolean :tag "Hide tags with no matching me=
ssages"))
> -          (:hide-if-empty (boolean :tag "Hide if empty")))))
> -    `(list (const :tag "" notmuch-hello-insert-tags-section)
> -          (plist :inline t :options ,opts))))
> +  :type '(list (const :tag "" notmuch-hello-insert-tags-section)
> +              (string :tag "Title for this section")
> +              (string :tag "Filter for each tag")
> +              (string :tag "Different filter to generate counts")
> +              (repeat :tag "Tags that will be hidden" string)
> +              (boolean :tag "Hide this on startup?")
> +              (boolean :tag "Hide tags with no matching messages")
> +              (boolean :tag "Hide if empty")))
>=20=20

> -(defun notmuch-hello-insert-tags-section (&rest options)
> +(defun notmuch-hello-insert-tags-section (title &optional filter filter-=
count hide-tags initially-hidden
> +                                               hide-empty-searches hide-=
if-empty)

This would make customization via elisp somewhat more cumbersome though,
because then one has to specify all preceding optional arguments to set
an option near the end of the argument list. Also, it gets harder to
decipher which value belongs to which option.

An alternative would be to force specifying all possible options in the
customization interface while still producing a plist as a
result. Something along these lines should work:

:type '(list (const :tag "" notmuch-hello-insert-tags-section)
             (const :tag "" :make-query)
             (string :tag "Filter for this tag")
             ...)

I guess the format instructions for the elements would have to be
tweaked a bit more to avoid excessive spacing due to the many hidden
elements.

Cheers,
Daniel

--=-=-=
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)

iQIcBAEBAgAGBQJOF48nAAoJEIaTAtce+Z+JW04P/isduyqNGGXYhCbRxp31iI4B
mOaGzcijIWaX9VwTV3X5JGR36EG6mkdj0/N9RuaAcMG7eBMTAhVIKhltqv7qWcPA
/Ybgo3OVEHPNPvEbb8W033ddOsCOPxZaFVdlY5xv50eI8byI8fKzryeYZGql32Gd
NwvbcCA7aUEkonz+yEniHBN60G1tv8lt4Xl9ONPx4PGxuhIwQJddT1xp29u11Voq
C29d+n5YgOiG2VueFj73q0G7l+992MWds6EIYIDV5tlua162CEYYEAAVbTEGSDMo
a+93aq69RqCHBx9nIU4/pIpNd/CS9iL00zPbN5jycYWVIxyQzCgJf9/0Qc0B4cJ4
ZRwXYnkTwjaeOAxPLQyqlq38yXQFEUXyyzsf32y82gw3ZcgS77rFyGeEfhwDRVPD
oLkbXr9pvpLn8HMsN2lCCwvuY3FSmn0obtVVy8z6q0Wj6HGKyebAVxmzPzyLlsjx
Pac/9QqqzSi86ZbHVuZpZybnzGyAD0R8LZdxYNdxj9lkY9Lbwaf9zw22hmh6YNyw
8vSGiNhkhYACjC3WQuXHTSn27kclDKjFXnQpdFyyQyX/1rEgEN3X9HA1AKuJ6xOw
+SANR0BRhwX1MWQTovafi6CJAw6KocL95zQ6NiAvNaTKnp+zOa73tURwFW8uuZWy
zsgPlVpP0wh0ZTWmgIZL
=cCJj
-----END PGP SIGNATURE-----
--=-=-=--