Re: [PATCH 1/5] emacs: compile and load notmuch-pick.el if present.

[Mark Walters <markwalters1009@gmail.com>]

On Wed, 25 Jul 2012, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> On Wed, Jul 25 2012, Mark Walters <markwalters1009@gmail.com> wrote:
>
>> Compile and load notmuch-pick.el if present.
>>
>> All the actual setup of pick is done in the function notmuch-pick-init
>> so we call that in the notmuch init function if it is bound. This
>> function will setup all extra keybinding etc.
>
> Great stuff! See a few thoughts below...

Thanks for these: I think they are all clear improvements.

>
>> ---
>>  emacs/Makefile.local |    3 ++-
>>  emacs/notmuch.el     |    5 +++++
>>  2 files changed, 7 insertions(+), 1 deletions(-)
>>
>> diff --git a/emacs/Makefile.local b/emacs/Makefile.local
>> index fb82247..9f4dba6 100644
>> --- a/emacs/Makefile.local
>> +++ b/emacs/Makefile.local
>> @@ -15,7 +15,8 @@ emacs_sources := \
>>  	$(dir)/notmuch-crypto.el \
>>  	$(dir)/notmuch-tag.el \
>>  	$(dir)/coolj.el \
>> -	$(dir)/notmuch-print.el
>> +	$(dir)/notmuch-print.el \
>> +	$(wildcard $(dir)/notmuch-pick.el)
>
> I wonder whether having this conditional is good idea. What if
> someone copies (or (sym)links) notmuch-pick.el there and then
> compiles and takes to use. Next time he takes clean tree and
> forgets to do this copying and installs to the same destination.
> Now there is old notmuch-pick.elc which might be out of sync.
>
> I think it would be better to provide a shell script in 
> notmuch-pick directory which byte-compiles and installs notmuch-pick
> in case user wants to install/update notmuch-pick. Whenever notmuch-pick
> is good enough to be shipped inside $(dir) above then the aboce conditional
> is not needed (at all).
>

I have now done this: I actually use a Makefile so that I can pick up
the config from notmuch: in particular the install directory
for the notmuch-pick.elc file. Does that seem reasonable?


>>  emacs_images := \
>>  	$(srcdir)/$(dir)/notmuch-logo.png
>> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
>> index fd1836f..4f3da4f 100644
>> --- a/emacs/notmuch.el
>> +++ b/emacs/notmuch.el
>> @@ -59,6 +59,9 @@
>>  (require 'notmuch-maildir-fcc)
>>  (require 'notmuch-message)
>>  
>> +;; Load notmuch-pick if available (but do not error if not present).
>> +(load "notmuch-pick" t)
>> +
>>  (defcustom notmuch-search-result-format
>>    `(("date" . "%12s ")
>>      ("count" . "%-7s ")
>> @@ -1088,6 +1091,8 @@ current search results AND that are tagged with the given tag."
>>  (defun notmuch ()
>>    "Run notmuch and display saved searches, known tags, etc."
>>    (interactive)
>> +  (when (fboundp 'notmuch-pick-init)
>> +    (notmuch-pick-init))
>>    (notmuch-hello))
>
> Instead of this could notmuch-pick.el contain:
>
> In the beginning:
>
> (require 'notmuch-hello)
> (require 'notmuch-show)
> (require 'notmuch) ;; XXX ATM, as notmuch-search-mode-map is defined here
>
> And, at the end, before (provide 'notmuch-pick), execute the lines
> what currenty are contained in (notmuch-pick-init).
>
> Then, those who want to start using notmuch-pick at this time
> can (just) write the following in their .emacs:
>
> (require 'notmuch)
> (require 'notmuch-pick)

This works very nicely. And if we want a transitional period the
notmuch-pick could be in (dir) but not loaded automatically.

Many thanks for the excellent suggestions.

Mark