Re: [PATCH] emacs: User-defined sections in notmuch-hello

2011-06-09 Thread Austin Clements
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.scho...@googlemail.com wrote:
 On Fri, 27 May 2011 20:52:01 +0200, Daniel Schoepe 
 daniel.scho...@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.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] emacs: User-defined sections in notmuch-hello

2011-05-27 Thread Daniel Schoepe
I forgot to run the test suite with the previous patch, which
revealed some minor formatting problems, updated patch in the
attachment.
From bca5a58d7910b6d46a782db787dfe07e2fcf21e1 Mon Sep 17 00:00:00 2001
From: Daniel Schoepe daniel.scho...@googlemail.com
Date: Thu, 26 May 2011 23:03:22 +0200
Subject: [PATCH] emacs: User-defined sections in notmuch-hello

This patch allows the user to define various sections that will
be displayed in notmuch-hello. I tried to keep the section
description flexible, so it allows different queries for the
tag buttons and the counts next to them, as well as hiding items
with zero results.

The sections are (hopefully) fully editable using customize.

I have not yet rewritten the saved-searches portion using this
mechanism, to avoid breaking the configurations of many users.
---
 emacs/notmuch-hello.el |  328 ++--
 emacs/notmuch-lib.el   |   22 ++--
 2 files changed, 275 insertions(+), 75 deletions(-)

diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
index 916cda1..40333ae 100644
--- a/emacs/notmuch-hello.el
+++ b/emacs/notmuch-hello.el
@@ -55,24 +55,115 @@
   :type 'boolean
   :group 'notmuch)
 
-(defcustom notmuch-hello-tag-list-make-query nil
-  Function or string to generate queries for the all tags list.
-
-This variable controls which query results are shown for each tag
-in the \all tags\ list. If nil, it will use all messages with
-that tag. If this is set to a string, it is used as a filter for
-messages having that tag (equivalent to \tag:TAG and (THIS-VARIABLE)\).
-Finally this can be a function that will be called for each tag and
-should return a filter for that tag, or nil to hide the tag.
-  :type '(choice (const :tag All messages nil)
-		 (const :tag Unread messages tag:unread)
-		 (const :tag Custom filter string)
-		 (const :tag Custom filter function function))
-  :group 'notmuch)
-
-(defcustom notmuch-hello-hide-tags nil
-  List of tags to be hidden in the \all tags\-section.
-  :type '(repeat string)
+(defvar notmuch-hello-section-all-tags
+  (list :title All tags:
+	:type 'eachtag)
+  Default section definition for the \all tags\ section.)
+
+(defvar notmuch-hello-section-unread-tags
+  (list :title All tags, unread only:
+	:type 'eachtag
+	:make-query tag:unread)
+  Show and count only unread messages for each tag.)
+
+(defvar notmuch-hello-hidden-sections nil
+  Sections that are hidden in notmuch-hello)
+
+(defvar notmuch-hello-first-run t
+  Internal variable for hiding sections with :hidden-on-startup)
+
+(define-widget 'notmuch-section-query-type 'lazy
+  Customize-type for query items
+  :tag Query item
+  :type '(list (string :tag Search title)
+	   (string :tag Query to use)
+	   (choice :tag Query for message count
+		   (const :tag Use previous query nil)
+		   (string :tag Different query
+
+(define-widget 'notmuch-section-make-query-type 'lazy
+  Customize-type for query functions
+  :tag Query function
+  :type '(choice (const :tag All messages with tag nil)
+		 (const :tag Only unread messages tag:unread)
+		 (string :tag Custom filter string)
+		 (function :tag Custom filter function)))
+
+(define-widget 'notmuch-section-type 'lazy
+  Customize-type for sections
+  :tag Custom section
+  :type
+  '(plist :options (((const :tag Title for this section :title)
+		 string)
+		((const :tag Type of this section :type)
+		 (choice (const :tag One item for each tag
+eachtag)
+			 (const :tag Custom list of searches
+query-list)))
+		((const :tag Function to generate a query, ignored if custom list
+			:make-query)
+		 notmuch-section-make-query)
+		((const :tag Function to generate counts, ignored if custom list
+			:make-count)
+		 notmuch-section-make-query)
+		((const :tag Function to create titles for tag entries, ignored if custom list
+			:make-title)
+		 (choice (const :tag The tag itself nil)
+			 (function :tag Custom function)))
+		((const :tag List of tags to hide, ignored if custom list
+			:hide-tags)
+		 (repeat (string :tag tag)))
+		((const :tag Search queries, only used if custom list
+			:items)
+		 (repeat notmuch-section-query))
+		((const :tag Hide if there are no results :hide-empty)
+		 boolean)
+		((const :tag Where should this be positioned :position)
+		 (choice (const :tag Before the search input before)
+			 (const :tag After the search input after))
+
+(defcustom notmuch-hello-sections (list notmuch-hello-section-all-tags)
+  Sections to be displayed in notmuch-hello.
+
+This variable does not include the saved-searches section, which
+is handled separately.
+This variable should be a list of plists with the following
+possible properties:
+
+:title - The title of the section
+:type - Can be 'eachtag or 'query-list, If 'eachtag, generate one
+item for each tag, otherwise use a fixed set of items.
+The following 

Re: [PATCH] emacs: User-defined sections in notmuch-hello

2011-05-27 Thread Daniel Schoepe
Accidentally attached the old file again, sorry for the noise.
From cafc93e3364955cab70885fec740fee87bc3248e Mon Sep 17 00:00:00 2001
From: Daniel Schoepe daniel.scho...@googlemail.com
Date: Thu, 26 May 2011 23:03:22 +0200
Subject: [PATCH] emacs: User-defined sections in notmuch-hello

This patch allows the user to define various sections that will
be displayed in notmuch-hello. I tried to keep the section
description flexible, so it allows different queries for the
tag buttons and the counts next to them, as well as hiding items
with zero results.

The sections are (hopefully) fully editable using customize.

I have not yet rewritten the saved-searches portion using this
mechanism, to avoid breaking the configurations of many users.
---
 emacs/notmuch-hello.el |  333 
 emacs/notmuch-lib.el   |   22 +-
 test/emacs.expected-output/notmuch-hello   |2 +-
 .../notmuch-hello-no-saved-searches|2 +-
 .../emacs.expected-output/notmuch-hello-with-empty |2 +-
 5 files changed, 282 insertions(+), 79 deletions(-)

diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
index 916cda1..f014357 100644
--- a/emacs/notmuch-hello.el
+++ b/emacs/notmuch-hello.el
@@ -55,24 +55,115 @@
   :type 'boolean
   :group 'notmuch)
 
-(defcustom notmuch-hello-tag-list-make-query nil
-  Function or string to generate queries for the all tags list.
-
-This variable controls which query results are shown for each tag
-in the \all tags\ list. If nil, it will use all messages with
-that tag. If this is set to a string, it is used as a filter for
-messages having that tag (equivalent to \tag:TAG and (THIS-VARIABLE)\).
-Finally this can be a function that will be called for each tag and
-should return a filter for that tag, or nil to hide the tag.
-  :type '(choice (const :tag All messages nil)
-		 (const :tag Unread messages tag:unread)
-		 (const :tag Custom filter string)
-		 (const :tag Custom filter function function))
-  :group 'notmuch)
-
-(defcustom notmuch-hello-hide-tags nil
-  List of tags to be hidden in the \all tags\-section.
-  :type '(repeat string)
+(defvar notmuch-hello-section-all-tags
+  (list :title All tags:
+	:type 'eachtag)
+  Default section definition for the \all tags\ section.)
+
+(defvar notmuch-hello-section-unread-tags
+  (list :title All tags, unread only:
+	:type 'eachtag
+	:make-query tag:unread)
+  Show and count only unread messages for each tag.)
+
+(defvar notmuch-hello-hidden-sections nil
+  Sections that are hidden in notmuch-hello)
+
+(defvar notmuch-hello-first-run t
+  Internal variable for hiding sections with :hidden-on-startup)
+
+(define-widget 'notmuch-section-query-type 'lazy
+  Customize-type for query items
+  :tag Query item
+  :type '(list (string :tag Search title)
+	   (string :tag Query to use)
+	   (choice :tag Query for message count
+		   (const :tag Use previous query nil)
+		   (string :tag Different query
+
+(define-widget 'notmuch-section-make-query-type 'lazy
+  Customize-type for query functions
+  :tag Query function
+  :type '(choice (const :tag All messages with tag nil)
+		 (const :tag Only unread messages tag:unread)
+		 (string :tag Custom filter string)
+		 (function :tag Custom filter function)))
+
+(define-widget 'notmuch-section-type 'lazy
+  Customize-type for sections
+  :tag Custom section
+  :type
+  '(plist :options (((const :tag Title for this section :title)
+		 string)
+		((const :tag Type of this section :type)
+		 (choice (const :tag One item for each tag
+eachtag)
+			 (const :tag Custom list of searches
+query-list)))
+		((const :tag Function to generate a query, ignored if custom list
+			:make-query)
+		 notmuch-section-make-query)
+		((const :tag Function to generate counts, ignored if custom list
+			:make-count)
+		 notmuch-section-make-query)
+		((const :tag Function to create titles for tag entries, ignored if custom list
+			:make-title)
+		 (choice (const :tag The tag itself nil)
+			 (function :tag Custom function)))
+		((const :tag List of tags to hide, ignored if custom list
+			:hide-tags)
+		 (repeat (string :tag tag)))
+		((const :tag Search queries, only used if custom list
+			:items)
+		 (repeat notmuch-section-query))
+		((const :tag Hide if there are no results :hide-empty)
+		 boolean)
+		((const :tag Where should this be positioned :position)
+		 (choice (const :tag Before the search input before)
+			 (const :tag After the search input after))
+
+(defcustom notmuch-hello-sections (list notmuch-hello-section-all-tags)
+  Sections to be displayed in notmuch-hello.
+
+This variable does not include the saved-searches section, which
+is handled separately.
+This variable should be a list of plists with the following
+possible properties:
+
+:title - The title of the section
+:type - Can