Re: [PATCH v2] emacs: display tags in notmuch-show with links

[Damien Cassou <damien.cassou@gmail.com>]

On Wed, Nov 14, 2012 at 1:41 PM, Mark Walters <markwalters1009@gmail.com> wrote:
> On the loading library issues I defer to Adam and Tomi.


I think they are right and I will try to make my code do something
good by default and do something better if header-button is detected.


> I do have a couple of comments on the current version
> though. First the patch is very big. I would prefer it split into
> several pieces


do you want me to send totally separate emails or do you want me to
send 1 discussion of multiple emails (with 0/4, 1/4, 2/4,...) ?


> something like:
>
> 1) Add tags to the headerline but not clickable


makes perfect sense


> 2) Add the header-button library if appropriate


I will try the other approach of not embedding the library in notmuch


> 3) Add notmuch-tagger and change headerline to buttonize the tags


makes perfect sense. The patch will buttonize the links if
header-button is present and nothing otherwise.


> 4) Add the tests.

ok


+ 5) Add buttons to tags in the body


>> +(defun notmuch-tagger-present-tags (tags &optional headerline)
>> +  "Return a property list which nicely presents all TAGS.
>> +
>> +If HEADERLINE is non-nil the returned list will be ready for
>> +inclusion in the buffer's header-line. HEADERLINE must be nil in
>> +all other cases."
>
> I find it odd to say what it returns if HEADERLINE is non-nil and then
> say otherwise HEADERLINE must be nil. Could you say what it returns in
> the nil case? Something along them lines of
>
> "if HEADERLINE is non-nil the returned will be ready for inclusion in
> the buffer's header-line (i.e., will use header-buttons if
> available). Otherwise it returns a list?? ready for inclusion in a
> buffer."

thank you for the comment, I will fix it and propose new patches


--
Damien Cassou
http://damiencassou.seasidehosting.st

"Success is the ability to go from one failure to another without
losing enthusiasm."
Winston Churchill