DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma;
	h=mime-version:sender:in-reply-to:references:date
	:x-google-sender-auth:message-id:subject:from:to:cc:content-type;
	b=TQFTspxWCCoTH+TIW8JLg0hlHpHkvjR/iumKNZ0YmczHIoUIyX7juHAvCgsev3aswS
	HMCpcqwge+3DzJlInpn72zDvE0LvTXEd740LQT2reMyVRU64Vum2eC/cJT/8MV05uErB
	JVfyyMnk/ib3XckzlIKgCuDqs7nzYa71bkuWY=
MIME-Version: 1.0
Sender: amdragon@gmail.com
In-Reply-To: <87tyc7hvtz.fsf@gilead.invalid>
References: <874o4g563y.fsf@gilead.invalid>
	<87tyc7hvtz.fsf@gilead.invalid>
Date: Fri, 10 Jun 2011 01:42:07 -0400
Message-ID: <BANLkTinKCLg1epsGvfxtyy-_G2Hm=gjUMQ@mail.gmail.com>
Subject: Re: [PATCH] emacs: User-defined sections in notmuch-hello
From: Austin Clements <amdragon@mit.edu>
To: Daniel Schoepe <daniel.schoepe@googlemail.com>
Content-Type: text/plain; charset=ISO-8859-1
Cc: notmuch@notmuchmail.org
Precedence: list

This looks really interesting.

I haven't examined the code very closely, but I have some high level comments.

It seems that the code is simultaneously trying to do something very
general, but also hard-coding a lot of behaviors, and I think the
code's complexity suffers for it.  What would this look like if the
*entire* hello screen were replaced by a configurable list of
sections?  What I'm imagining could be as simple as a list of
section-generating functions plus a collection of standard functions
for generating standard sections (probably known to customize to make
it easy for non-elispers to control).

For more-configurable sections, there could be a mechanism for passing
arguments, but this quickly enters the land of hair-raising defcustoms
and confusing flexibility.  I think it would be much simpler and more
user-friendly to require the functions in this list to take no
arguments (at least, none that are user-configurable).  Flexible
sections could be implemented then as a low-level function that takes
arguments and one or more no-argument high-level functions that call
the low-level section generator either with fixed arguments, or with
the values of other customize variables.  This would keep things
simple and fairly flexible for non-elispers, without sacrificing
complete flexibility if you're willing to write a few lines of elisp.

Maybe you also want to configure the title of each section.  But, IMO,
this is also confusing flexibility.  In fact, with the no-argument
section generators, it would make a lot of sense to simply put the
section name in the function's plist.

This wouldn't fit well with the current logic to compute the
cross-section widest tag, but personally I've always found that
behavior extremely confusing (not to mention buggy) and wouldn't miss
it at all.

The use of the term "title" for pretty-printed tags is confusing.  To
me, "title" means the section title.  For example, I couldn't figure
out what "Return widest title string in SECTION." meant until I read
the code (*flashback* "a section only has one title, what the heck is
the widest one?")

This patch should probably be split into a few patches, as it seems to
also introduce new functionality for some of the sections (and does
little things like moving notmuch-remove-if-not).

On Fri, Jun 3, 2011 at 9:46 AM, Daniel Schoepe
<daniel.schoepe@googlemail.com> wrote:
> On Fri, 27 May 2011 20:52:01 +0200, Daniel Schoepe <daniel.schoepe@googlemail.com> wrote:
>> I'll also work on some tests for this functionality, (if no one
>> has big, structural complaints about the code).
>
> I rebased my patch against the current master and wrote some tests.