From: Mark Walters <markwalters1009@gmail.com>
To: Austin Clements <amdragon@MIT.EDU>, notmuch@notmuchmail.org
Subject: Re: [PATCH 2/4] emacs: Record part p-list in a text property
In-Reply-To: <1369687594-31774-3-git-send-email-amdragon@mit.edu>
References: <1369687594-31774-1-git-send-email-amdragon@mit.edu>
	<1369687594-31774-3-git-send-email-amdragon@mit.edu>
User-Agent: Notmuch/0.14+255~gff3cc55 (http://notmuchmail.org) Emacs/23.4.1
	(i486-pc-linux-gnu)
Date: Mon, 27 May 2013 23:19:18 +0100
Message-ID: <8738t8jcbd.fsf@qmul.ac.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list


A couple of small nits:

Austin Clements <amdragon@MIT.EDU> writes:

> This is similar to what we already do with the message p-list, though
> we apply the part's text property to the whole part's text, in
> contrast with the message p-list, which is (rather obscurely) only
> applied to the first character.
> ---
>  emacs/notmuch-lib.el  |   16 ++++++++++++++++
>  emacs/notmuch-show.el |   13 ++++++++++++-
>  2 files changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
> index 790136e..09ce25e 100644
> --- a/emacs/notmuch-lib.el
> +++ b/emacs/notmuch-lib.el
> @@ -360,6 +360,22 @@ OBJECT."
>     below
>     string))
>  
> +(defun notmuch-put-text-property-if-nil (start end property value
> +					       &optional object)
> +  "Like `put-text-property', but only set the property where it is nil."
> +  (while (< start end)
> +    (let ((start-nil (text-property-any start end property nil object)))
> +      (if (null start-nil)
> +	  ;; There are no more nil regions; exit the loop
> +	  (setq start end)
> +	;; Find the end of the nil region
> +	(let ((end-nil
> +	       (or (text-property-not-all start-nil end property nil object)
> +		   end)))
> +	  ;; Set the property
> +	  (put-text-property start-nil end-nil property value object)
> +	  (setq start end-nil))))))
> +
>  (defun notmuch-logged-error (msg &optional extra)
>    "Log MSG and EXTRA to *Notmuch errors* and signal MSG.
>  
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index a080134..acd0b55 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -900,7 +900,10 @@ 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 msg beg (point) hide)
> +    ;; Record part information.  Since we already inserted subparts,
> +    ;; don't override exiting :notmuch-part properties.
                         ^^^
existing

> +    (notmuch-put-text-property-if-nil beg (point) :notmuch-part part)))
>  
>  (defun notmuch-show-insert-body (msg body depth)
>    "Insert the body BODY at depth DEPTH in the current thread."
> @@ -1404,6 +1407,14 @@ Some useful entries are:
>      (notmuch-show-move-to-message-top)
>      (get-text-property (point) :notmuch-message-properties)))
>  
> +(defun notmuch-show-get-part-properties ()
> +  "Return the properties of the part containing point.
> +
> +This is the part property list retrieved from the CLI.  Signals
> +an error if there is no part containing point."

I think something here should say innermost part or something similar.

Best wishes

Mark


> +  (or (get-text-property (point) :notmuch-part)
> +      (error "No message part here")))
> +
>  (defun notmuch-show-set-prop (prop val &optional props)
>    (let ((inhibit-read-only t)
>  	(props (or props
> -- 
> 1.7.10.4