Re: [PATCH v4 1/2] emacs: User-defined sections in notmuch-hello

[Daniel Schoepe <daniel.schoepe@googlemail.com>]

[-- Attachment #1: Type: text/plain, Size: 2754 bytes --]

On Sat, 09 Jul 2011 01:00:03 +0200, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
> 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.
> 
> I think that the customization interface is much better arranged when it
> is modified like in the patch below (it is against v3).
> 
> [..]
> -  :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 messages"))
> -          (: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")))
>  

> -(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

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]