From: Michal Sojka <sojkam1@fel.cvut.cz>
To: Daniel Schoepe <daniel.schoepe@googlemail.com>, notmuch@notmuchmail.org
Subject: Re: [PATCH v4 1/2] emacs: User-defined sections in notmuch-hello
In-Reply-To: <87mxgo2wns.fsf@gilead.invalid>
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> <87mxgo2wns.fsf@gilead.invalid>
User-Agent: Notmuch/0.5-332-gf8bc48d (http://notmuchmail.org) Emacs/23.3.1
	(x86_64-pc-linux-gnu)
Date: Sat, 09 Jul 2011 07:35:46 +0200
Message-ID: <87sjqg6mod.fsf@steelpick.2x.cz>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list

On Fri, 08 Jul 2011, Daniel Schoepe wrote:
Non-text part: multipart/signed
> 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.

Not necessarily. In fact, notmuch-hello-insert-tags-section is only a
thin wrapper of notmuch-hello-insert-searches. In elisp you can always
use directly those plist-based "low-level" functions.

-Michal