From: Tomi Ollila <tomi.ollila@iki.fi>
To: Michal Sojka <sojkam1@fel.cvut.cz>, notmuch@notmuchmail.org
Subject: Re: [PATCH v2] emacs: Add customization group for replying
In-Reply-To: <87y5kntjf1.fsf@steelpick.2x.cz>
References: <1346919815-20397-1-git-send-email-sojkam1@fel.cvut.cz>
	<m2wr07fs0y.fsf@guru.guru-group.fi>
	<87y5kntjf1.fsf@steelpick.2x.cz>
User-Agent: Notmuch/0.14+11~gd9bf007 (http://notmuchmail.org) Emacs/24.2.1
	(x86_64-unknown-linux-gnu)
Date: Thu, 06 Sep 2012 17:00:55 +0300
Message-ID: <m2sjavxnc8.fsf@guru.guru-group.fi>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: list

On Thu, Sep 06 2012, Michal Sojka <sojkam1@fel.cvut.cz> wrote:

>> So far good, but...
>>
>> You forgot to handle the current group setting for 
>> notmuch-message-replied-tags -- currently notmuch-message.el has this:
>>
>> (defcustom notmuch-message-replied-tags '("replied")
>>   "Tags to be automatically added to or removed from a message when it is
>>   replied to.
>> Any tag in the list will be added to a replied message or,
>> if it is prefaced with a \"-\", removed.
>>
>> For example, if you wanted to add a \"replied\" tag and remove
>> the \"inbox\" and \"todo\", you would set
>>     (\"replied\" \"-inbox\" \"-todo\"\)"
>>   :type 'list
>>   :group 'notmuch-send)
>>
>> If the above was changed to 'normuch-send, (require 'notmuch-lib)
>> was added to the beginning of notmuch-message.el and the defgroup
>> below was written as
>>
>> (defgroup notmuch-reply
>>   '((message-insertion custom-group)))
>>    "Replying to messages.
>>     ...
>>
>> would that work as expected ?
>
> Hi Tomi,
>
> I don't really get what are you trying to say. Do you mean that
> notmuch-message-replied-tags is referenced from notmuch-lib.el without
> (require 'notmuch-message)? I'm not sure whether it is required, but it
> works here without problems.

Ok, some sillines in my part and something I did not check well
enough before replying...

First, notmuch-lib.el defines notmuch-send group, yet 
(defcustom notmuch-message-replied-tags ...) is defined 
in notmuch-message.el which is not requiring notmuch-lib...
and all of this works now...

Actually, notmuch-lib.el defines all defgroups (except coolj).

Then, I found the following: notmuch-mua-send-hook is defined 
both in notmuch-hooks and notmuch-send groups (having a precedent
helps a bit there when choosing alternatives below)


All the defgroups are defined using pattern:

(defgroup notmuch-<group> nil ...)

We should follow this pattern unless there is good reason not to 
do so here: notmuch-send is written as:

  (defgroup notmuch-send nil
    "Sending messages from Notmuch."
    :group 'notmuch)

  (custom-add-to-group 'notmuch-send 'message 'custom-group)

But, as the documentation in this notmuch-reply definition references
'Message insertion' group, doing

  (defgroup notmuch-reply 
    '((message-insertion custom-group))'
    "Replying to messages.

  Most of the reply customization settings can be found in `Message
  insertion' group. Notmuch specific settings are included directly
  here."
  :group 'notmuch)

could be good thing to do here; definition matches doc from the beginning
(just not adding any of the custom variables, yet).


Now, currently notmuch-message.el defines notmuch-message-replied-tags
and puts that to :group 'notmuch-send

There are 2 alternatives:

1) change that to :group 'notmuch-reply

2) add (custom-add-to-group 'notmuch-reply 'notmuch-message-replied-tags 'custom-variable)
   to notmuch-message.el (or notmuch-lib.el)

2b) change :group 'notmuch-send -> :group 'notmuch-reply and
   add (custom-add-to-group 'notmuch-send 'notmuch-message-replied-tags 'custom-variable)
   to notmuch-message.el (or notmuch-lib.el)


I personally would slightly lean to option 1 but if 2 gets more 
supporters then 2b (so that the spesific group is in the group definition).

On other words; 1 or 2b gets LGTM from me :)

Understood ? :D


Tomi


> Running M-x customize-variable RET notmuch-message-replied-tags RET
> shows the following. Note that both groups are mentioned at the bottom.
> No warning or errors are produced.

Yes, I tested that missing group-thing with:

(defgroup foo '((bar-group custom-group)) "test")

It seems the nonexisting bar-group is created on-the-fly (but
exist temporarily). No warnings nor errors :).

>
> -Michal

Tomi

>
> --8<---------------cut here---------------start------------->8---
> To apply changes, use the Save or Set buttons.
> For details, see Saving Customizations in the Emacs manual.
>
>                                           Search 
>
>  Operate on all settings in this buffer:
>   Set for current session   Save for future sessions 
>   Undo edits   Reset to saved   Erase customizations     Exit 
>
> Hide notmuch-message-replied-tags: '("replied")
>     State : STANDARD. (mismatch)
>    Tags to be automatically added to or removed from a message when it is replied to. Hide   
>    Any tag in the list will be added to a replied message or,
>    if it is prefaced with a "-", removed.
>    
>    For example, if you wanted to add a "replied" tag and remove
>    the "inbox" and "todo", you would set
>        ("replied" "-inbox" "-todo")
> Groups: Notmuch Send Notmuch Reply
> --8<---------------cut here---------------end--------------->8---