Re: [PATCH 1/6] emacs: add helper for tag change list manipulation

Subject: Re: [PATCH 1/6] emacs: add helper for tag change list manipulation

Date: Thu, 06 Sep 2012 17:30:56 +0300

To: Jani Nikula, Michal Nazarewicz, notmuch@notmuchmail.org

Cc:

From: Tomi Ollila


On Mon, Sep 03 2012, Jani Nikula wrote:

> On Mon, 03 Sep 2012, Michal Nazarewicz <mina86@mina86.com> wrote:
>> Jani Nikula <jani@nikula.org> writes:
>>> Add a helper to create (and optionally reverse) a list of tag changes.
>>> ---
>>>  emacs/notmuch-tag.el |   17 +++++++++++++++++
>>>  1 file changed, 17 insertions(+)
>>>
>>> diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
>>> index 0c0fc87..c1aeb99 100644
>>> --- a/emacs/notmuch-tag.el
>>> +++ b/emacs/notmuch-tag.el
>>> @@ -140,6 +140,23 @@ notmuch-after-tag-hook will be run."
>>>    ;; in all cases we return tag-changes as a list
>>>    tag-changes)
>>>  
>>> +(defun notmuch-tag-change-list (tags &optional reverse)
>>> +  "Convert TAGS into a list of tag changes.
>>> +
>>> +Add a \"+\" prefix to any tag in TAGS list that doesn't already
>>> +begin with a \"+\" or a \"-\". If REVERSE is non-nil, replace all
>>> +\"+\" prefixes with \"-\" and vice versa in the result."
>>> +  (mapcar (lambda (str)
>>> +	    (let ((s (if (not (string-match "^[+-]" str))
>>> +			 (concat "+" str)
>>> +		       str)))
>>
>> (if (string-match "^[-+]" str) str (concat "+" str))
>>
>> Negation only makes things less readable IMO.
>
> Agreed.

The whole series LGTM -- but are you still going to do 
changes here ?

> Jani.

Tomi


>
>>
>>> +	      (if reverse
>>> +		  (concat (if (= (string-to-char s) ?-) "+" "-")
>>> +			  (substring s 1))
>>> +		s)))
>>> +	  tags))
>>> +
>>> +
>>>  ;;
>>>  
>>>  (provide 'notmuch-tag)
>>
>> -- 
>> Best regards,                                         _     _
>> .o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
>> ..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
>> ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--

Thread: