From: Tomi Ollila <tomi.ollila@iki.fi>
To: Mark Walters <markwalters1009@gmail.com>, notmuch@notmuchmail.org
Subject: Re: [PATCH] contrib: pick: unset current-prefix-arg for notmuch-show
In-Reply-To: <1380718788-18165-1-git-send-email-markwalters1009@gmail.com>
References: <1380718788-18165-1-git-send-email-markwalters1009@gmail.com>
User-Agent: Notmuch/0.16+87~g451fefe (http://notmuchmail.org) Emacs/24.3.1
	(x86_64-unknown-linux-gnu)
Date: Wed, 02 Oct 2013 19:57:51 +0300
Message-ID: <m2siwjhapc.fsf@guru.guru-group.fi>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: list

On Wed, Oct 02 2013, Mark Walters <markwalters1009@gmail.com> wrote:

> notmuch-show looks at the prefix argument directly (ie via
> current-prefix-arg) rather than using some form of the interactive
> command. Thus we need to unset current-prefix-arg before calling notmuch-show.
> ---

Alternatively fix notmuch-show to do

(interactive "sNotmuch show: \nP")

and then add new argument, like `elide-toggle` or something to the
function.

Not considering notmuch-pick there is only one (1) call to that function,
inside notmuch sources... soo the change there would be.

--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -495 +495 @@ If BARE is set then do not prefix with \"thread:\""
-       (notmuch-show thread-id
+       (notmuch-show thread-id nil

IMO there should not be "current-prefix-arg" side effects when calling 
notmuch-show non-interactively, yet other opinions welcome.


Tomi


> I was just doing final tests for pick to mainline when I discovered
> the following bug. The show-out option was not working quite correctly
> (it was eliding the non-matching messages). I traced this to the
> following: notmuch-show looks at current-prefix-arg directly. The
> recent change to call notmuch-pick-show-message-out via a prefix arg
> rather than its own keybinding meant that this prefix arg is being
> passed on to the subsidary notmuch-show call. 
>
> To avoid this we unset current-prefix-arg before calling
> notmuch-show. It seems neatest to do this in
> notmuch-pick-show-message.
>
> Sorry for not spotting this before.
>
> Best wishes
>
> Mark
>
>
>
>
>
>  contrib/notmuch-pick/notmuch-pick.el |   11 +++++++----
>  1 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/contrib/notmuch-pick/notmuch-pick.el b/contrib/notmuch-pick/notmuch-pick.el
> index 0aa651e..6b727b2 100644
> --- a/contrib/notmuch-pick/notmuch-pick.el
> +++ b/contrib/notmuch-pick/notmuch-pick.el
> @@ -488,10 +488,13 @@ Does NOT change the database."
>  Shows in split pane or whole window according to value of
>  `notmuch-pick-show-out'. A prefix argument reverses the choice."
>    (interactive "P")
> -  (if (or (and notmuch-pick-show-out  (not arg))
> -	  (and (not notmuch-pick-show-out) arg))
> -      (notmuch-pick-show-message-out)
> -    (notmuch-pick-show-message-in)))
> +  ;; We have to unset current-prefix-arg as notmuch-show examines it
> +  ;; directly.
> +  (let ((current-prefix-arg nil))
> +    (if (or (and notmuch-pick-show-out (not arg))
> +	    (and (not notmuch-pick-show-out) arg))
> +	(notmuch-pick-show-message-out)
> +      (notmuch-pick-show-message-in))))
>  
>  (defun notmuch-pick-scroll-message-window ()
>    "Scroll the message window (if it exists)"
> -- 
> 1.7.9.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch