From: Austin Clements <aclements@csail.mit.edu>
To: Mark Walters <markwalters1009@gmail.com>, notmuch@notmuchmail.org,
	Adam Wolfe Gordon <awg+notmuch@xvx.ca>
Subject: Re: [PATCH v3 3/5] emacs: show: pass button to create-overlays
In-Reply-To: <1370024806-6616-4-git-send-email-markwalters1009@gmail.com>
References: <1370024806-6616-1-git-send-email-markwalters1009@gmail.com>
	<1370024806-6616-4-git-send-email-markwalters1009@gmail.com>
User-Agent: Notmuch/0.15.2+83~g8bee3c4 (http://notmuchmail.org) Emacs/23.4.1
	(i486-pc-linux-gnu)
Date: Fri, 31 May 2013 18:59:48 -0400
Message-ID: <874ndi3gd7.fsf@awakening.csail.mit.edu>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list

On Fri, 31 May 2013, Mark Walters <markwalters1009@gmail.com> wrote:
> Now that the bodypart code has the button we can pass that to
> create-overlays and simplify that.
> ---
>  emacs/notmuch-show.el |   26 ++++++++++++--------------
>  1 file changed, 12 insertions(+), 14 deletions(-)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index be3aeac..66cc3a5 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -851,21 +851,19 @@ message at DEPTH in the current thread."
>        (setq handlers (cdr handlers))))
>    t)
>  
> -(defun notmuch-show-create-part-overlays (msg beg end hide)
> +(defun notmuch-show-create-part-overlays (button beg end hide)
>    "Add an overlay to the part between BEG and END"
> -  (let* ((button (button-at beg))
> -	 (part-beg (and button (1+ (button-end button)))))
> -
> -    ;; If the part contains no text we do not make it toggleable. We
> -    ;; also need to check that the button is a genuine part button not
> -    ;; a notmuch-wash button.
> -    (when (and button (/= part-beg end) (button-get button :base-label))
> -      (button-put button 'overlay (make-overlay part-beg end))
> -      ;; We toggle the button for hidden parts as that gets the
> +
> +  ;; If there is no button (i.e., the part is text/plain and the first
> +  ;; part) or if the part has no content then we don't make the part
> +  ;; toggleable.
> +  (when (and button (/= beg end))
> +    (button-put button 'overlay (make-overlay beg end))
> +    ;; We toggle the button for hidden parts as that gets the
>        ;; button label right.
>        (save-excursion
>  	(when hide
> -	  (notmuch-show-toggle-part-invisibility button))))))
> +	  (notmuch-show-toggle-part-invisibility button)))))

I might just be mis-following the diff here, but shouldn't the above get
re-indented?

>  
>  (defun notmuch-show-insert-bodypart (msg part depth &optional hide)
>    "Insert the body part PART at depth DEPTH in the current thread.
> @@ -879,10 +877,10 @@ If HIDE is non-nil then initially hide this part."
>  			     "text/x-diff")
>  			content-type))
>  	 (nth (plist-get part :id))
> -	 (beg (point))
>  	 ;; We omit the part button for the first (or only) part if this is text/plain.
>  	 (button (unless (and (string= mime-type "text/plain") (<= nth 1))
> -		   (notmuch-show-insert-part-header nth mime-type content-type (plist-get part :filename)))))
> +		   (notmuch-show-insert-part-header nth mime-type content-type (plist-get part :filename))))
> +	 (beg (point)))

Was this swap necessary?

>  
>      (notmuch-show-insert-bodypart-internal msg part mime-type nth depth button)
>      ;; Some of the body part handlers leave point somewhere up in the
> @@ -891,7 +889,7 @@ If HIDE is non-nil then initially hide this part."
>      ;; Ensure that the part ends with a carriage return.
>      (unless (bolp)
>        (insert "\n"))
> -    (notmuch-show-create-part-overlays msg beg (point) hide)))
> +    (notmuch-show-create-part-overlays button beg (point) hide)))
>  
>  (defun notmuch-show-insert-body (msg body depth)
>    "Insert the body BODY at depth DEPTH in the current thread."
> -- 
> 1.7.10.4