Re: [PATCH v1 2/2] emacs: Bind "u" to jump to the parent message in a thread

Subject: Re: [PATCH v1 2/2] emacs: Bind "u" to jump to the parent message in a thread

Date: Thu, 29 Aug 2019 21:50:26 +0300

To: David Edmondson, notmuch@notmuchmail.org

Cc:

From: Tomi Ollila


On Thu, Aug 29 2019, David Edmondson wrote:

> ---
>  emacs/notmuch-show.el | 55 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 1e3834f2..7c50d2cd 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -1173,6 +1173,60 @@ is t, hide the part initially and show the button."
>    (setq notmuch-show--forest forest)
>    (mapc (lambda (thread) (notmuch-show-insert-thread thread 0)) forest))
>  
> +(defun notmuch-show--find-parent-in-thread-node (thread-node id)
> +  (let* ((msg (car thread-node))
> +	 (reply-nodes (cadr thread-node))
> +	 ;; Is one of the replies to this message the one we are looking
> +	 ;; for?
> +	 (candidate
> +	  (catch 'found
> +	    (mapc (lambda (reply-node)
> +		    (let* ((reply-msg (car reply-node))
> +			   (reply-id (plist-get reply-msg :id)))
> +		      (when (string= reply-id id)
> +			(throw 'found (plist-get msg :id)))))
> +		  reply-nodes)
> +	    nil)))
> +    (if candidate
> +	candidate
> +      ;; Otherwise recurse down.
> +      (notmuch-show--find-parent-in-thread reply-nodes id))))
> +
> +(defun notmuch-show--find-parent-in-thread (thread id)
> +  (let ((msg (catch 'found
> +	       (mapc (lambda (thread-node)
> +		       (setq msg (notmuch-show--find-parent-in-thread-node thread-node id))
> +		       (when msg
> +			 (throw 'found msg)))
> +		     thread)
> +	       nil)))
> +    msg))
> +
> +(defun notmuch-show--find-parent-in-thread-set (thread-set id)
> +  (let ((msg (catch 'found
> +	       (mapc (lambda (thread)
> +		       (setq msg (notmuch-show--find-parent-in-thread thread id))
> +		       (when msg
> +			 (throw 'found msg)))
> +		     thread-set)
> +	       nil)))
> +    msg))
> +
> +(defun notmuch-show--find-parent ()
> +  "Find the parent of the current message."
> +  (notmuch-show--find-parent-in-thread-set notmuch-show--forest
> +					   (notmuch-show-get-message-id t)))
> +
> +(defun notmuch-show--show-parent ()
> +  "Jump to the parent of the current message, opening it if necessary."
> +  (interactive)
> +  (let ((parent (notmuch-show--find-parent)))
> +    (unless parent
> +      (error "The current message has no parent."))

Would it be nicer to just message if there is no parent. do we have 
similar (error) cases in after any other interactive command ?

Otherwise this looks good to me. I'd guess forest data structure
is robuse enough for these call paths to work...

dme: What do you think could be done differently to possibly
get less expensive implementation ?

Tomi

> +    (notmuch-show-goto-message (notmuch-id-to-query parent))
> +    (unless (notmuch-show-message-visible-p)
> +      (notmuch-show-toggle-message))))
> +
>  (defvar notmuch-id-regexp
>    (concat
>     ;; Match the id: prefix only if it begins a word (to disallow, for
> @@ -1516,6 +1570,7 @@ reset based on the original query."
>      (define-key map "t" 'toggle-truncate-lines)
>      (define-key map "." 'notmuch-show-part-map)
>      (define-key map "B" 'notmuch-show-browse-urls)
> +    (define-key map "u" 'notmuch-show--show-parent)
>      map)
>    "Keymap for \"notmuch show\" buffers.")
>  (fset 'notmuch-show-mode-map notmuch-show-mode-map)
> -- 
> 2.22.0
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch
_______________________________________________
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Thread: