From: Mark Walters <markwalters1009@gmail.com>
To: Austin Clements <amdragon@MIT.EDU>
Subject: Re: [PATCH 1/2] emacs: show: use interactive instead of
	current-prefix-arg
In-Reply-To: <20131013033157.GE10539@mit.edu>
References: <1381348886-5673-1-git-send-email-markwalters1009@gmail.com>
	<1381348886-5673-2-git-send-email-markwalters1009@gmail.com>
	<20131013033157.GE10539@mit.edu>
User-Agent: Notmuch/0.15.2+334~gafd5c6a (http://notmuchmail.org) Emacs/23.4.1
	(x86_64-pc-linux-gnu)
Date: Sun, 13 Oct 2013 08:25:07 +0100
Message-ID: <8761t13a5o.fsf@qmul.ac.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: notmuch@notmuchmail.org
Precedence: list


On Sun, 13 Oct 2013, Austin Clements <amdragon@MIT.EDU> wrote:
> Code looks good.  Two minor comments below.

That's great. 
>
> Quoth Mark Walters on Oct 09 at  9:01 pm:
>> Currently notmuch-show looks at the prefix-arg directly via
>> current-prefix-arg. This changes it to use the interactive
>> specification.
>> ---
>>  emacs/notmuch-show.el |   12 +++++++-----
>>  emacs/notmuch.el      |    5 +++--
>>  2 files changed, 10 insertions(+), 7 deletions(-)
>> 
>> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
>> index 5d7e24b..15f9ce6 100644
>> --- a/emacs/notmuch-show.el
>> +++ b/emacs/notmuch-show.el
>> @@ -1080,15 +1080,17 @@ buttons for a corresponding notmuch search."
>>  	(make-text-button (first link) (second link)
>>  			  :type 'notmuch-button-type
>>  			  'action `(lambda (arg)
>> -				     (notmuch-show ,(third link)))
>> +				     (notmuch-show ,(third link) current-prefix-arg))
>
> Hmm.  Does this work?  Can you activate the button with C-u RET?

This does work: you can activate the button with C-u RET and it does
elide the resulting the thread. (This is identical to the current
behaviour)


>
>>  			  'follow-link t
>>  			  'help-echo "Mouse-1, RET: search for this message"
>>  			  'face goto-address-mail-face)))))
>>  
>>  ;;;###autoload
>> -(defun notmuch-show (thread-id &optional parent-buffer query-context buffer-name)
>> +(defun notmuch-show (thread-id &optional elide-toggle parent-buffer query-context buffer-name)
>>    "Run \"notmuch show\" with the given thread ID and display results.
>>  
>> +The optional ELIDE-TOGGLE inverts the default elide behaviour.
>
> "ELIDE-TOGGLE, if non-nil, inverts the default elide behavior."?

I will fix this.

Thanks for the review

Mark

>
>> +
>>  The optional PARENT-BUFFER is the notmuch-search buffer from
>>  which this notmuch-show command was executed, (so that the
>>  next thread from that buffer can be show when done with this
>> @@ -1102,7 +1104,7 @@ The optional BUFFER-NAME provides the name of the buffer in
>>  which the message thread is shown. If it is nil (which occurs
>>  when the command is called interactively) the argument to the
>>  function is used."
>> -  (interactive "sNotmuch show: ")
>> +  (interactive "sNotmuch show: \nP")
>>    (let ((buffer-name (generate-new-buffer-name
>>  		      (or buffer-name
>>  			  (concat "*notmuch-" thread-id "*")))))
>> @@ -1112,9 +1114,9 @@ function is used."
>>      (setq notmuch-show-process-crypto notmuch-crypto-process-mime)
>>      ;; Set the default value for
>>      ;; `notmuch-show-elide-non-matching-messages' in this buffer. If
>> -    ;; there is a prefix argument, invert the default.
>> +    ;; elide-toggle is set, invert the default.
>>      (setq notmuch-show-elide-non-matching-messages notmuch-show-only-matching-messages)
>> -    (if current-prefix-arg
>> +    (if elide-toggle
>>  	(setq notmuch-show-elide-non-matching-messages (not notmuch-show-elide-non-matching-messages)))
>>  
>>      (setq notmuch-show-thread-id thread-id
>> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
>> index 0ff248b..7124d0a 100644
>> --- a/emacs/notmuch.el
>> +++ b/emacs/notmuch.el
>> @@ -486,13 +486,14 @@ If BARE is set then do not prefix with \"thread:\""
>>    "Return a list of authors for the current region"
>>    (notmuch-search-properties-in-region :subject beg end))
>>  
>> -(defun notmuch-search-show-thread ()
>> +(defun notmuch-search-show-thread (&optional elide-toggle)
>>    "Display the currently selected thread."
>> -  (interactive)
>> +  (interactive "P")
>>    (let ((thread-id (notmuch-search-find-thread-id))
>>  	(subject (notmuch-search-find-subject)))
>>      (if (> (length thread-id) 0)
>>  	(notmuch-show thread-id
>> +		      elide-toggle
>>  		      (current-buffer)
>>  		      notmuch-search-query-string
>>  		      ;; Name the buffer based on the subject.