From: Mark Walters <markwalters1009@gmail.com>
To: Austin Clements <amdragon@MIT.EDU>, notmuch@notmuchmail.org
Subject: Re: [PATCH 3/7] emacs: Update tags by rewriting the search result
	line in place
In-Reply-To: <1342140319-19859-4-git-send-email-amdragon@mit.edu>
References: <1342140319-19859-1-git-send-email-amdragon@mit.edu>
	<1342140319-19859-4-git-send-email-amdragon@mit.edu>
User-Agent: Notmuch/0.13.2+63~g1397edd (http://notmuchmail.org) Emacs/23.4.1
	(x86_64-pc-linux-gnu)
Date: Fri, 13 Jul 2012 18:57:03 +0100
Message-ID: <87a9z38ryo.fsf@qmul.ac.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list

On Fri, 13 Jul 2012, Austin Clements <amdragon@MIT.EDU> wrote:
> Now that we keep the full thread result object, we can refresh a
> result after any changes by simply deleting and reconstructing the
> result line from scratch.
>
> A convenient side-effect of this wholesale replacement is that search
> now re-applies notmuch-search-line-faces when tags change.
> ---
>  emacs/notmuch.el |   55 ++++++++++++++++++++++++++++++------------------------
>  1 file changed, 31 insertions(+), 24 deletions(-)
>
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index 82c148d..2f83425 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -509,28 +509,12 @@ and will also appear in a buffer named \"*Notmuch errors*\"."
>  	    (error (buffer-substring beg end))
>  	    ))))))
>  
> -(defun notmuch-search-set-tags (tags)
> -  (save-excursion
> -    (end-of-line)
> -    (re-search-backward "(")
> -    (forward-char)
> -    (let ((beg (point))
> -	  (inhibit-read-only t))
> -      (re-search-forward ")")
> -      (backward-char)
> -      (let ((end (point)))
> -	(delete-region beg end)
> -	(insert (propertize (mapconcat  'identity tags " ")
> -			    'face 'notmuch-tag-face))))))
> -
> -(defun notmuch-search-get-tags ()
> -  (save-excursion
> -    (end-of-line)
> -    (re-search-backward "(")
> -    (let ((beg (+ (point) 1)))
> -      (re-search-forward ")")
> -      (let ((end (- (point) 1)))
> -	(split-string (buffer-substring-no-properties beg end))))))
> +(defun notmuch-search-set-tags (tags &optional pos)
> +  (let ((new-result (plist-put (notmuch-search-get-result pos) :tags tags)))
> +    (notmuch-search-update-result new-result pos)))
> +
> +(defun notmuch-search-get-tags (&optional pos)
> +  (plist-get (notmuch-search-get-result pos) :tags))
>  
>  (defun notmuch-search-get-tags-region (beg end)
>    (save-excursion
> @@ -583,6 +567,29 @@ This function advances the next thread when finished."
>    (notmuch-search-tag '("-inbox"))
>    (notmuch-search-next-thread))
>  
> +(defun notmuch-search-update-result (result &optional pos)
> +  "Update the result object of the current thread and redraw it."

I think this comment reads a little awkwardly. Maybe "Replace the result
object of the current thread by RESULT and redraw it"?

> +  (let ((start (notmuch-search-result-beginning pos))
> +	(end (notmuch-search-result-end pos))
> +	(init-point (point))
> +	(inhibit-read-only t))
> +    ;; Delete the current thread
> +    (delete-region start end)
> +    ;; Insert the updated thread
> +    (notmuch-search-show-result result start)
> +    ;; There may have been markers pointing into the text we just
> +    ;; replaced.  For the most part, there's nothing we can do about
> +    ;; this, but we can fix markers that were at point (which includes
> +    ;; point itself and any save-excursions for which point hasn't
> +    ;; moved) by re-inserting the text that should come before point
> +    ;; before markers.
> +    (when (and (>= init-point start) (<= init-point end))
> +      (let* ((new-end (notmuch-search-result-end start))
> +	     (new-point (if (= init-point end)
> +			    new-end
> +			  (min init-point (- new-end 1)))))
> +	(insert-before-markers (delete-and-extract-region start new-point))))))

I think this doesn't always do quite the right thing with multiline
results: the one that I can reproduce reliably is if you are on the
second line of the first result, and the first line of this result is
the first visible line in the window, and update it (eg by marking
unread) then the window scrolls so that the second line of the first
result is the first line visible in the window (ie the window scrolls up
one line)

> +
>  (defun notmuch-search-process-sentinel (proc msg)
>    "Add a message to let user know when \"notmuch search\" exits"
>    (let ((buffer (process-buffer proc))
> @@ -745,10 +752,10 @@ non-authors is found, assume that all of the authors match."
>  			 (mapconcat 'identity (plist-get result :tags) " ")
>  			 'font-lock-face 'notmuch-tag-face) ")")))))
>  
> -(defun notmuch-search-show-result (result)
> +(defun notmuch-search-show-result (result &optional pos)


Perhaps a documentation line? along the lines of Insert RESULT in buffer
at POS or the end of buffer.

Best wishes

Mark

>    ;; Ignore excluded matches
>    (unless (= (plist-get result :matched) 0)
> -    (let ((beg (point-max)))
> +    (let ((beg (or pos (point-max))))
>        (save-excursion
>  	(goto-char beg)
>  	(dolist (spec notmuch-search-result-format)
> -- 
> 1.7.10
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch