Date: Sat, 14 Jan 2012 13:08:28 -0500
From: Austin Clements <amdragon@MIT.EDU>
To: Pieter Praet <pieter@praet.org>
Subject: Re: [PATCH] emacs: logically group def{custom,face}s
Message-ID: <20120114180828.GD1801@mit.edu>
References: <87ty3ypsgp.fsf@praet.org>
	<1326531898-17356-1-git-send-email-pieter@praet.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1326531898-17356-1-git-send-email-pieter@praet.org>
User-Agent: Mutt/1.5.21 (2010-09-15)
Cc: Notmuch Mail <notmuch@notmuchmail.org>
Precedence: list

Quoth Pieter Praet on Jan 14 at 10:04 am:
> To allow for expansion whilst keeping everything tidy and organized,
> move all defcustom/defface variables to the following subgroups,
> defined in notmuch-lib.el:
> 
> - Hello
> - Search
> - Show
> - Send
> - Crypto
> - Hooks
> - External Commands
> - Appearance
> 
> As an added benefit, defcustom keyword args are now consistently
> in order of appearance @ defcustom's docstring (OCD much?).

Thanks for doing this.  I recently went into customize-group notmuch
and was overwhelmed by the pile of options presented.

> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
> index 0f856bf..f6f48e9 100644
> --- a/emacs/notmuch-lib.el
> +++ b/emacs/notmuch-lib.el
> @@ -28,17 +28,58 @@
>    "Notmuch mail reader for Emacs."
>    :group 'mail)
>  

Group docstrings aren't generally of the form "Options concerning
..."; they just jump into what they concern.  E.g.,

Convenience : Convenience features for faster editing.

Calendar Hooks : Calendar hooks.

Also, all but one of the tags you give the groups would be
automatically derived by Emacs, so you can remove those.

> +(defgroup notmuch-hello nil
> +  "Options concerning `notmuch-hello-mode'."
> +  :tag "Notmuch Hello"
> +  :group 'notmuch)

Perhaps "Overview of saved searches, tags, etc."

> +
> +(defgroup notmuch-search nil
> +  "Options concerning `notmuch-search-mode'."
> +  :tag "Notmuch Search"
> +  :group 'notmuch)

"Searching and sorting mail"?

> +
> +(defgroup notmuch-show nil
> +  "Options concerning `notmuch-show-mode'."
> +  :tag "Notmuch Show"
> +  :group 'notmuch)

"Showing messages and threads"?

> +
> +(defgroup notmuch-send nil
> +  "Options concerning the sending of messages."
> +  :tag "Notmuch Send"
> +  :group 'notmuch)

"Sending messages from Notmuch"?

We should probably link to the 'message group, perhaps by adding
  :link '(custom-group-link message)
here or maybe to the notmuch group itself.  Unfortunately, I don't
think you can actually add a group to another group after it's been
defined (though I could be wrong).

> +
> +(defgroup notmuch-crypto nil
> +  "Options concerning the processing and fontification of
> +cryptographic MIME parts in `notmuch-show-mode'."
> +  :tag "Notmuch Crypto"
> +  :group 'notmuch)

"Processing and display of cryptographic MIME parts"?  (You also don't
want the docstring to be too long, given how it's displayed.)

> +
> +(defgroup notmuch-hooks nil
> +  "Run custom code on well-defined occasions."
> +  :tag "Notmuch Hooks"
> +  :group 'notmuch)
> +
> +(defgroup notmuch-external nil
> +  "Run more custom code on different well-defined occasions."
> +  :tag "Notmuch External Commands"
> +  :group 'notmuch)

Oof!  It's okay to be a little redundant in the docstring.  Core Emacs
options do it.  "External commands"?

> +
> +(defgroup notmuch-appearance nil
> +  "Options concerning how Notmuch looks."
> +  :tag "Notmuch Appearance"
> +  :group 'notmuch)

"How Notmuch looks"?

I worry that notmuch-appearance is a catch-all that most options
arguably fit in to.  In particular, some notmuch-show options are also
in this group and some aren't and it's not clear to me what the rule
is.

Perhaps this should be notmuch-faces and limited to just faces (and
maybe options that aren't technically faces but that affect face
selection)?  Then the grouping rule would be obvious, like it is for
all of the other groups.