From: Austin Clements <aclements@csail.mit.edu>
To: Mark Walters <markwalters1009@gmail.com>, notmuch@notmuchmail.org
Subject: Re: [PATCH v2 3/3] emacs: show: implement lazy hidden part handling
In-Reply-To: <1369555061-21361-4-git-send-email-markwalters1009@gmail.com>
References: <1369555061-21361-1-git-send-email-markwalters1009@gmail.com>
	<1369555061-21361-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: Thu, 30 May 2013 19:28:18 -0400
Message-ID: <877gig2gkt.fsf@awakening.csail.mit.edu>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list

On Sun, 26 May 2013, Mark Walters <markwalters1009@gmail.com> wrote:
> This adds the actual code to do the lazy insertion of hidden parts.
>
> We use a memory inefficient but simple method: when we come to insert
> the part if it is hidden we just store all of the arguments to the
> part insertion function as a button property. This means when we want
> to show the part we can just resume where we left off.
>
> The only slight subtlety/hack is that to simplify the handling of the
> invisibility overlay (for the hiding unhiding later) we do insert some
> dummy text which we remove when we show the part.
> ---
>  emacs/notmuch-show.el |   32 ++++++++++++++++++++++++++++++--
>  1 files changed, 30 insertions(+), 2 deletions(-)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 591ad56..94faa9b 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -560,6 +560,7 @@ message at DEPTH in the current thread."
>  	 (overlay (button-get button 'overlay)))
>      (when overlay
>        (let* ((show (overlay-get overlay 'invisible))
> +	     (lazy-part (button-get button :notmuch-lazy-part))
>  	     (new-start (button-start button))
>  	     (button-label (button-get button :base-label))
>  	     (old-point (point))
> @@ -570,7 +571,11 @@ message at DEPTH in the current thread."
>  	(let ((old-end (button-end button)))
>  	  (move-overlay button new-start (point))
>  	  (delete-region (point) old-end))
> -	(goto-char (min old-point (1- (button-end button))))))))
> +	(goto-char (min old-point (1- (button-end button))))
> +	(when (and show lazy-part)
> +	  (save-excursion
> +	    (button-put button :notmuch-lazy-part nil)
> +	    (notmuch-show-lazy-part lazy-part button)))))))
>  
>  (defun notmuch-show-multipart/*-to-list (part)
>    (mapcar (lambda (inner-part) (plist-get inner-part :content-type))
> @@ -854,6 +859,24 @@ message at DEPTH in the current thread."
>        (setq handlers (cdr handlers))))
>    t)
>  
> +(defun notmuch-show-lazy-part (part-args button)
> +  (interactive)
> +  ;; We have to save the depth as we can't find the depth when narrowed
> +  (let ((inhibit-read-only t)
> +	(overlay (button-get button 'overlay))
> +	(depth (notmuch-show-get-depth)))
> +    (save-restriction
> +      (narrow-to-region (overlay-start overlay) (1- (overlay-end overlay)))
> +      (delete-region (overlay-start overlay) (1- (overlay-end overlay)))
> +      (goto-char (overlay-start overlay))
> +      (apply #'notmuch-show-insert-bodypart-internal (nconc part-args (list button)))

Is there a reason 'button' couldn't be included in parts-args when
parts-args was originally constructed below?  (If so, then this probably
deserves a comment.)

> +      (indent-rigidly (overlay-start overlay)
> +		      (1- (overlay-end overlay))
> +		      depth))
> +    ;; We deferred deleting this character to simplify handling of the
> +    ;; overlay: all of the above takes place inside the overlay.
> +    (delete-region (1- (overlay-end overlay)) (overlay-end overlay))))
> +
>  (defun notmuch-show-create-part-overlays (msg beg end hide)
>    "Add an overlay to the part between BEG and END"
>    (let* ((button (button-at beg))
> @@ -888,7 +911,12 @@ If HIDE is non-nil then initially hide this part."
>  	 (button (unless (string= mime-type "text/plain")
>  		   (notmuch-show-insert-part-header nth mime-type content-type (plist-get part :filename)))))
>  
> -    (notmuch-show-insert-bodypart-internal msg part mime-type nth depth content-type button)
> +    (if (not hide)
> +        (notmuch-show-insert-bodypart-internal msg part mime-type nth depth content-type button)
> +      (insert "lazy part")

The insert is worth a comment.  Or fixing if it's not too difficult.

> +      (button-put button :notmuch-lazy-part
> +                  (list msg part mime-type nth depth content-type)))
> +

To summarize the IRC discussion for the record, this changes the
behavior of lazily inserted parts that we can't render.  Currently the
only way we know if we can render a part is by attempting to render it.
As a result, lazily inserted parts that we can't render that would
previously have offered to save the part when the part button was
selected will now attempt to render the part and simply not display
anything.

>      ;; Some of the body part handlers leave point somewhere up in the
>      ;; part, so we make sure that we're down at the end.
>      (goto-char (point-max))
> -- 
> 1.7.9.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch