From: Tomi Ollila <tomi.ollila@iki.fi>
To: Mark Walters <markwalters1009@gmail.com>, notmuch@notmuchmail.org
Subject: Re: [PATCH 1/5] emacs: compile and load notmuch-pick.el if present.
In-Reply-To: <1343164911-31589-2-git-send-email-markwalters1009@gmail.com>
References: <1343164911-31589-1-git-send-email-markwalters1009@gmail.com>
	<1343164911-31589-2-git-send-email-markwalters1009@gmail.com>
User-Agent: Notmuch/0.13.2+97~g7dcb4ce (http://notmuchmail.org) Emacs/23.1.1
	(x86_64-redhat-linux-gnu)
Date: Wed, 25 Jul 2012 12:17:48 +0300
Message-ID: <m2ipdcqjwj.fsf@guru.guru-group.fi>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list

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...

> ---
>  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).


>  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)

>  
>  (defun notmuch-interesting-buffer (b)
> -- 
> 1.7.9.1

Tomi