unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH v2] (notmuch-show "id:doesnotexist") should not throw an error.
@ 2016-02-10 14:01 David Edmondson
  2016-02-10 14:01 ` [PATCH v2] emacs: Report a lack of matches when calling `notmuch-show' David Edmondson
  0 siblings, 1 reply; 6+ messages in thread
From: David Edmondson @ 2016-02-10 14:01 UTC (permalink / raw)
  To: notmuch


(notmuch-show "id:doesnotexist") should not throw an error.

This is a followup to id:"874nw0ltwz.fsf@praet.org" and
id:"cuntx91fwaa.fsf@hotblack-desiato.hh.sledj.net".

The originally proposed change (to have id: links call `notmuch-show'
rather than `notmuch-search') was already made, but the difficulties
with links that generated no results was not addressed. This patch
aims to do that.

v2:
- Re-worked to `ding' rather than throwing an error.
- Re-jig the relationship between `notmuch-show',
  `notmuch-show-refresh-view' and `notmuch-show--build-buffer'.
- Various buffer local variables no longer need to be `permanent-local'.


David Edmondson (1):
  emacs: Report a lack of matches when calling `notmuch-show'.

 emacs/notmuch-show.el | 144 +++++++++++++++++++++++++++++---------------------
 emacs/notmuch.el      |   6 ++-
 2 files changed, 89 insertions(+), 61 deletions(-)

-- 
2.1.4

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2] emacs: Report a lack of matches when calling `notmuch-show'.
  2016-02-10 14:01 [PATCH v2] (notmuch-show "id:doesnotexist") should not throw an error David Edmondson
@ 2016-02-10 14:01 ` David Edmondson
  2016-02-10 18:50   ` Mark Walters
  0 siblings, 1 reply; 6+ messages in thread
From: David Edmondson @ 2016-02-10 14:01 UTC (permalink / raw)
  To: notmuch

If the basic query passed to `notmuch-show' generates no results, ring
the bell and inform the user that no messages matched the query rather
than displaying an empty buffer and showing an obscure error.

Similarly when refreshing a `notmuch-show' buffer and no messages match.
---
 emacs/notmuch-show.el | 144 +++++++++++++++++++++++++++++---------------------
 emacs/notmuch.el      |   6 ++-
 2 files changed, 89 insertions(+), 61 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 3345878..9a74848 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -153,27 +153,21 @@ indentation."
 
 (defvar notmuch-show-thread-id nil)
 (make-variable-buffer-local 'notmuch-show-thread-id)
-(put 'notmuch-show-thread-id 'permanent-local t)
 
 (defvar notmuch-show-parent-buffer nil)
 (make-variable-buffer-local 'notmuch-show-parent-buffer)
-(put 'notmuch-show-parent-buffer 'permanent-local t)
 
 (defvar notmuch-show-query-context nil)
 (make-variable-buffer-local 'notmuch-show-query-context)
-(put 'notmuch-show-query-context 'permanent-local t)
 
 (defvar notmuch-show-process-crypto nil)
 (make-variable-buffer-local 'notmuch-show-process-crypto)
-(put 'notmuch-show-process-crypto 'permanent-local t)
 
 (defvar notmuch-show-elide-non-matching-messages nil)
 (make-variable-buffer-local 'notmuch-show-elide-non-matching-messages)
-(put 'notmuch-show-elide-non-matching-messages 'permanent-local t)
 
 (defvar notmuch-show-indent-content t)
 (make-variable-buffer-local 'notmuch-show-indent-content)
-(put 'notmuch-show-indent-content 'permanent-local t)
 
 (defvar notmuch-show-attachment-debug nil
   "If t log stdout and stderr from attachment handlers
@@ -1197,71 +1191,101 @@ non-nil.
 The optional BUFFER-NAME provides the name of the buffer in
 which the message thread is shown. If it is nil (which occurs
 when the command is called interactively) the argument to the
-function is used."
+function is used.
+
+Returns the buffer containing the messages, or NIL if no messages
+matched."
   (interactive "sNotmuch show: \nP")
   (let ((buffer-name (generate-new-buffer-name
 		      (or buffer-name
 			  (concat "*notmuch-" thread-id "*")))))
     (switch-to-buffer (get-buffer-create buffer-name))
-    ;; Set the default value for `notmuch-show-process-crypto' in this
-    ;; buffer.
-    (setq notmuch-show-process-crypto notmuch-crypto-process-mime)
-    ;; Set the default value for
-    ;; `notmuch-show-elide-non-matching-messages' in this buffer. If
-    ;; elide-toggle is set, invert the default.
-    (setq notmuch-show-elide-non-matching-messages notmuch-show-only-matching-messages)
-    (if elide-toggle
-	(setq notmuch-show-elide-non-matching-messages (not notmuch-show-elide-non-matching-messages)))
+    ;; No need to track undo information for this buffer.
+    (setq buffer-undo-list t)
+
+    (notmuch-show-mode)
 
+    ;; Set various buffer local variables to their appropriate initial
+    ;; state. Do this after enabling `notmuch-show-mode' so that they
+    ;; aren't wiped out.
     (setq notmuch-show-thread-id thread-id
 	  notmuch-show-parent-buffer parent-buffer
-	  notmuch-show-query-context query-context)
-    (notmuch-show-build-buffer)
-    (notmuch-show-goto-first-wanted-message)
-    (current-buffer)))
+	  notmuch-show-query-context query-context
 
-(defun notmuch-show-build-buffer ()
-  (let ((inhibit-read-only t))
+	  notmuch-show-process-crypto notmuch-crypto-process-mime
+	  ;; If `elide-toggle', invert the default value.
+	  notmuch-show-elide-non-matching-messages
+	  (if elide-toggle
+	      (not notmuch-show-only-matching-messages)
+	    notmuch-show-only-matching-messages))
 
-    (notmuch-show-mode)
     (add-hook 'post-command-hook #'notmuch-show-command-hook nil t)
-
-    ;; Don't track undo information for this buffer
-    (set 'buffer-undo-list t)
+    (jit-lock-register #'notmuch-show-buttonise-links)
 
     (notmuch-tag-clear-cache)
-    (erase-buffer)
-    (goto-char (point-min))
-    (save-excursion
-      (let* ((basic-args (list notmuch-show-thread-id))
-	     (args (if notmuch-show-query-context
-		       (append (list "\'") basic-args
-			       (list "and (" notmuch-show-query-context ")\'"))
-		     (append (list "\'") basic-args (list "\'"))))
-	     (cli-args (cons "--exclude=false"
-			     (when notmuch-show-elide-non-matching-messages
-			       (list "--entire-thread=false")))))
-
-	(notmuch-show-insert-forest (notmuch-query-get-threads (append cli-args args)))
-	;; If the query context reduced the results to nothing, run
-	;; the basic query.
-	(when (and (eq (buffer-size) 0)
-		   notmuch-show-query-context)
-	  (notmuch-show-insert-forest
-	   (notmuch-query-get-threads (append cli-args basic-args)))))
-
-      (jit-lock-register #'notmuch-show-buttonise-links)
-
-      (notmuch-show-mapc (lambda () (notmuch-show-set-prop :orig-tags (notmuch-show-get-tags))))
+
+    (let ((inhibit-read-only t))
+      (if (notmuch-show--build-buffer)
+	  ;; Messages were inserted into the buffer.
+	  (current-buffer)
+
+	;; No messages were inserted - presumably none matched the
+	;; query.
+	(kill-buffer (current-buffer))
+	(ding)
+	(message "No messages matched the query!")
+	nil))))
+
+(defun notmuch-show--build-buffer (&optional state)
+  "Display messages matching the current buffer context.
+
+Apply the previously saved STATE if supplied, otherwise show the
+first relevant message.
+
+If no messages match the query return NIL."
+  (let* ((basic-args (list notmuch-show-thread-id))
+	 (args (if notmuch-show-query-context
+		   (append (list "\'") basic-args
+			   (list "and (" notmuch-show-query-context ")\'"))
+		 (append (list "\'") basic-args (list "\'"))))
+	 (cli-args (cons "--exclude=false"
+			 (when notmuch-show-elide-non-matching-messages
+			   (list "--entire-thread=false"))))
+
+	 (forest (or (notmuch-query-get-threads (append cli-args args))
+		     ;; If a query context reduced the number of
+		     ;; results to zero, try again without it.
+		     (and notmuch-show-query-context
+			  (notmuch-query-get-threads (append cli-args basic-args)))))
+
+	 ;; Must be reset every time we are going to start inserting
+	 ;; messages into the buffer.
+	 (notmuch-show-previous-subject ""))
+
+    (when forest
+      (notmuch-show-insert-forest forest)
+
+      ;; Cache the original tags for each message so that we can display
+      ;; changes.
+      (notmuch-show-mapc
+       (lambda () (notmuch-show-set-prop :orig-tags (notmuch-show-get-tags))))
 
       ;; Set the header line to the subject of the first message.
       (setq header-line-format
 	    (replace-regexp-in-string "%" "%%"
-			    (notmuch-sanitize
-			     (notmuch-show-strip-re
-			      (notmuch-show-get-subject)))))
+				      (notmuch-sanitize
+				       (notmuch-show-strip-re
+					(notmuch-show-get-subject)))))
 
-      (run-hooks 'notmuch-show-hook))))
+      (run-hooks 'notmuch-show-hook)
+
+      (if state
+	  (notmuch-show-apply-state state)
+	;; With no state to apply, just go to the first message.
+	(notmuch-show-goto-first-wanted-message)))
+
+    ;; Report back to the caller whether any messages matched.
+    forest))
 
 (defun notmuch-show-capture-state ()
   "Capture the state of the current buffer.
@@ -1320,17 +1344,17 @@ reset based on the original query."
   (let ((inhibit-read-only t)
 	(state (unless reset-state
 		 (notmuch-show-capture-state))))
-    ;; erase-buffer does not seem to remove overlays, which can lead
+    ;; `erase-buffer' does not seem to remove overlays, which can lead
     ;; to weird effects such as remaining images, so remove them
     ;; manually.
     (remove-overlays)
     (erase-buffer)
-    (notmuch-show-build-buffer)
-    (if state
-	(notmuch-show-apply-state state)
-      ;; We're resetting state, so navigate to the first open message
-      ;; and mark it read, just like opening a new show buffer.
-      (notmuch-show-goto-first-wanted-message))))
+
+    (unless (notmuch-show--build-buffer state)
+      ;; No messages were inserted.
+      (kill-buffer (current-buffer))
+      (ding)
+      (message "Refreshing the buffer resulted in no messages!"))))
 
 (defvar notmuch-show-stash-map
   (let ((map (make-sparse-keymap)))
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 463b926..3100b97 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -458,7 +458,11 @@ no messages in the region then return nil."
   (notmuch-search-properties-in-region :subject beg end))
 
 (defun notmuch-search-show-thread (&optional elide-toggle)
-  "Display the currently selected thread."
+  "Display the currently selected thread.
+
+With a prefix argument, invert the default value of
+`notmuch-show-only-matching-messages' when displaying the
+thread."
   (interactive "P")
   (let ((thread-id (notmuch-search-find-thread-id))
 	(subject (notmuch-search-find-subject)))
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] emacs: Report a lack of matches when calling `notmuch-show'.
  2016-02-10 14:01 ` [PATCH v2] emacs: Report a lack of matches when calling `notmuch-show' David Edmondson
@ 2016-02-10 18:50   ` Mark Walters
  2016-02-10 19:52     ` David Edmondson
  0 siblings, 1 reply; 6+ messages in thread
From: Mark Walters @ 2016-02-10 18:50 UTC (permalink / raw)
  To: David Edmondson, notmuch



On Wed, 10 Feb 2016, David Edmondson <dme@dme.org> wrote:
> If the basic query passed to `notmuch-show' generates no results, ring
> the bell and inform the user that no messages matched the query rather
> than displaying an empty buffer and showing an obscure error.
>
> Similarly when refreshing a `notmuch-show' buffer and no messages match.

This basically looks fine to me and all tests pass. The code movement
and cleanup all looks fine.  Two minor things, one tiny nit below; and I
wonder whether just having the buffer say "No search results" (or
something similar) and leave the user to kill it would be nicer than
dinging (and more in line with the way search and tree behave).

[In some sense I think this way is right and search and tree are wrong,
but that is probably difficult to get round as search and tree run
asynchronously.]

Best wishes

Mark


> ---
>  emacs/notmuch-show.el | 144 +++++++++++++++++++++++++++++---------------------
>  emacs/notmuch.el      |   6 ++-
>  2 files changed, 89 insertions(+), 61 deletions(-)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 3345878..9a74848 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -153,27 +153,21 @@ indentation."
>  
>  (defvar notmuch-show-thread-id nil)
>  (make-variable-buffer-local 'notmuch-show-thread-id)
> -(put 'notmuch-show-thread-id 'permanent-local t)
>  
>  (defvar notmuch-show-parent-buffer nil)
>  (make-variable-buffer-local 'notmuch-show-parent-buffer)
> -(put 'notmuch-show-parent-buffer 'permanent-local t)
>  
>  (defvar notmuch-show-query-context nil)
>  (make-variable-buffer-local 'notmuch-show-query-context)
> -(put 'notmuch-show-query-context 'permanent-local t)
>  
>  (defvar notmuch-show-process-crypto nil)
>  (make-variable-buffer-local 'notmuch-show-process-crypto)
> -(put 'notmuch-show-process-crypto 'permanent-local t)
>  
>  (defvar notmuch-show-elide-non-matching-messages nil)
>  (make-variable-buffer-local 'notmuch-show-elide-non-matching-messages)
> -(put 'notmuch-show-elide-non-matching-messages 'permanent-local t)
>  
>  (defvar notmuch-show-indent-content t)
>  (make-variable-buffer-local 'notmuch-show-indent-content)
> -(put 'notmuch-show-indent-content 'permanent-local t)
>  
>  (defvar notmuch-show-attachment-debug nil
>    "If t log stdout and stderr from attachment handlers
> @@ -1197,71 +1191,101 @@ non-nil.
>  The optional BUFFER-NAME provides the name of the buffer in
>  which the message thread is shown. If it is nil (which occurs
>  when the command is called interactively) the argument to the
> -function is used."
> +function is used.
> +
> +Returns the buffer containing the messages, or NIL if no messages
> +matched."
>    (interactive "sNotmuch show: \nP")
>    (let ((buffer-name (generate-new-buffer-name
>  		      (or buffer-name
>  			  (concat "*notmuch-" thread-id "*")))))
>      (switch-to-buffer (get-buffer-create buffer-name))
> -    ;; Set the default value for `notmuch-show-process-crypto' in this
> -    ;; buffer.
> -    (setq notmuch-show-process-crypto notmuch-crypto-process-mime)
> -    ;; Set the default value for
> -    ;; `notmuch-show-elide-non-matching-messages' in this buffer. If
> -    ;; elide-toggle is set, invert the default.
> -    (setq notmuch-show-elide-non-matching-messages notmuch-show-only-matching-messages)
> -    (if elide-toggle
> -	(setq notmuch-show-elide-non-matching-messages (not notmuch-show-elide-non-matching-messages)))
> +    ;; No need to track undo information for this buffer.
> +    (setq buffer-undo-list t)
> +
> +    (notmuch-show-mode)
>  
> +    ;; Set various buffer local variables to their appropriate initial
> +    ;; state. Do this after enabling `notmuch-show-mode' so that they
> +    ;; aren't wiped out.
>      (setq notmuch-show-thread-id thread-id
>  	  notmuch-show-parent-buffer parent-buffer
> -	  notmuch-show-query-context query-context)
> -    (notmuch-show-build-buffer)
> -    (notmuch-show-goto-first-wanted-message)
> -    (current-buffer)))
> +	  notmuch-show-query-context query-context
>  
> -(defun notmuch-show-build-buffer ()
> -  (let ((inhibit-read-only t))
> +	  notmuch-show-process-crypto notmuch-crypto-process-mime
> +	  ;; If `elide-toggle', invert the default value.
> +	  notmuch-show-elide-non-matching-messages
> +	  (if elide-toggle
> +	      (not notmuch-show-only-matching-messages)
> +	    notmuch-show-only-matching-messages))
>  
> -    (notmuch-show-mode)
>      (add-hook 'post-command-hook #'notmuch-show-command-hook nil t)
> -
> -    ;; Don't track undo information for this buffer
> -    (set 'buffer-undo-list t)
> +    (jit-lock-register #'notmuch-show-buttonise-links)
>  
>      (notmuch-tag-clear-cache)
> -    (erase-buffer)
> -    (goto-char (point-min))
> -    (save-excursion
> -      (let* ((basic-args (list notmuch-show-thread-id))
> -	     (args (if notmuch-show-query-context
> -		       (append (list "\'") basic-args
> -			       (list "and (" notmuch-show-query-context ")\'"))
> -		     (append (list "\'") basic-args (list "\'"))))
> -	     (cli-args (cons "--exclude=false"
> -			     (when notmuch-show-elide-non-matching-messages
> -			       (list "--entire-thread=false")))))
> -
> -	(notmuch-show-insert-forest (notmuch-query-get-threads (append cli-args args)))
> -	;; If the query context reduced the results to nothing, run
> -	;; the basic query.
> -	(when (and (eq (buffer-size) 0)
> -		   notmuch-show-query-context)
> -	  (notmuch-show-insert-forest
> -	   (notmuch-query-get-threads (append cli-args basic-args)))))
> -
> -      (jit-lock-register #'notmuch-show-buttonise-links)
> -
> -      (notmuch-show-mapc (lambda () (notmuch-show-set-prop :orig-tags (notmuch-show-get-tags))))
> +
> +    (let ((inhibit-read-only t))
> +      (if (notmuch-show--build-buffer)
> +	  ;; Messages were inserted into the buffer.
> +	  (current-buffer)
> +
> +	;; No messages were inserted - presumably none matched the
> +	;; query.
> +	(kill-buffer (current-buffer))
> +	(ding)
> +	(message "No messages matched the query!")
> +	nil))))
> +
> +(defun notmuch-show--build-buffer (&optional state)
> +  "Display messages matching the current buffer context.
> +
> +Apply the previously saved STATE if supplied, otherwise show the
> +first relevant message.
> +
> +If no messages match the query return NIL."
> +  (let* ((basic-args (list notmuch-show-thread-id))
> +	 (args (if notmuch-show-query-context
> +		   (append (list "\'") basic-args
> +			   (list "and (" notmuch-show-query-context ")\'"))
> +		 (append (list "\'") basic-args (list "\'"))))
> +	 (cli-args (cons "--exclude=false"
> +			 (when notmuch-show-elide-non-matching-messages
> +			   (list "--entire-thread=false"))))
> +
> +	 (forest (or (notmuch-query-get-threads (append cli-args args))
> +		     ;; If a query context reduced the number of
> +		     ;; results to zero, try again without it.
> +		     (and notmuch-show-query-context
> +			  (notmuch-query-get-threads (append cli-args basic-args)))))
> +
> +	 ;; Must be reset every time we are going to start inserting
> +	 ;; messages into the buffer.
> +	 (notmuch-show-previous-subject ""))
> +
> +    (when forest
> +      (notmuch-show-insert-forest forest)
> +
> +      ;; Cache the original tags for each message so that we can display
> +      ;; changes.

^^ I think "Store the original tags for each message" would be better,
particularly as this is nothing to do with the tag cache as used by say
notmuch-tag-clear-cache.


> +      (notmuch-show-mapc
> +       (lambda () (notmuch-show-set-prop :orig-tags (notmuch-show-get-tags))))
>  
>        ;; Set the header line to the subject of the first message.
>        (setq header-line-format
>  	    (replace-regexp-in-string "%" "%%"
> -			    (notmuch-sanitize
> -			     (notmuch-show-strip-re
> -			      (notmuch-show-get-subject)))))
> +				      (notmuch-sanitize
> +				       (notmuch-show-strip-re
> +					(notmuch-show-get-subject)))))
>  
> -      (run-hooks 'notmuch-show-hook))))
> +      (run-hooks 'notmuch-show-hook)
> +
> +      (if state
> +	  (notmuch-show-apply-state state)
> +	;; With no state to apply, just go to the first message.
> +	(notmuch-show-goto-first-wanted-message)))
> +
> +    ;; Report back to the caller whether any messages matched.
> +    forest))
>  
>  (defun notmuch-show-capture-state ()
>    "Capture the state of the current buffer.
> @@ -1320,17 +1344,17 @@ reset based on the original query."
>    (let ((inhibit-read-only t)
>  	(state (unless reset-state
>  		 (notmuch-show-capture-state))))
> -    ;; erase-buffer does not seem to remove overlays, which can lead
> +    ;; `erase-buffer' does not seem to remove overlays, which can lead
>      ;; to weird effects such as remaining images, so remove them
>      ;; manually.
>      (remove-overlays)
>      (erase-buffer)
> -    (notmuch-show-build-buffer)
> -    (if state
> -	(notmuch-show-apply-state state)
> -      ;; We're resetting state, so navigate to the first open message
> -      ;; and mark it read, just like opening a new show buffer.
> -      (notmuch-show-goto-first-wanted-message))))
> +
> +    (unless (notmuch-show--build-buffer state)
> +      ;; No messages were inserted.
> +      (kill-buffer (current-buffer))
> +      (ding)
> +      (message "Refreshing the buffer resulted in no messages!"))))
>  
>  (defvar notmuch-show-stash-map
>    (let ((map (make-sparse-keymap)))
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index 463b926..3100b97 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -458,7 +458,11 @@ no messages in the region then return nil."
>    (notmuch-search-properties-in-region :subject beg end))
>  
>  (defun notmuch-search-show-thread (&optional elide-toggle)
> -  "Display the currently selected thread."
> +  "Display the currently selected thread.
> +
> +With a prefix argument, invert the default value of
> +`notmuch-show-only-matching-messages' when displaying the
> +thread."
>    (interactive "P")
>    (let ((thread-id (notmuch-search-find-thread-id))
>  	(subject (notmuch-search-find-subject)))
> -- 
> 2.1.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] emacs: Report a lack of matches when calling `notmuch-show'.
  2016-02-10 18:50   ` Mark Walters
@ 2016-02-10 19:52     ` David Edmondson
  2016-02-10 20:14       ` David Edmondson
  2016-02-10 21:05       ` Mark Walters
  0 siblings, 2 replies; 6+ messages in thread
From: David Edmondson @ 2016-02-10 19:52 UTC (permalink / raw)
  To: Mark Walters, notmuch

On Wed, Feb 10 2016, Mark Walters wrote:
> This basically looks fine to me and all tests pass. The code movement
> and cleanup all looks fine.

Thanks.

> Two minor things, one tiny nit below; and I wonder whether just having
> the buffer say "No search results" (or something similar) and leave
> the user to kill it would be nicer than dinging (and more in line with
> the way search and tree behave).
>
> [In some sense I think this way is right and search and tree are wrong,
> but that is probably difficult to get round as search and tree run
> asynchronously.]

What if we did "notmuch count $query" first in the search and tree case,
and did the "(ding) (message ...)" thing if the count returned 0? (Just
wondering about whether having everything behave that way would be
possible and acceptable.)

The original impetus for this change was someone who hits an id: button
that is either a false match (i.e it wasn't ever intended to be a
notmuch reference) or for a message that they don't have. In both of
those cases popping up a buffer that says only "No match." would be
annoying. If we were considering the case where people are using "M-x
notmuch-show", it seems less clear on the right thing to do, but overall
I prefer this approach to the useless buffer that I have to kill/quit.

>> +      ;; Cache the original tags for each message so that we can display
>> +      ;; changes.
>
> ^^ I think "Store the original tags for each message" would be better,
> particularly as this is nothing to do with the tag cache as used by say
> notmuch-tag-clear-cache.

Agreed - fixed.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] emacs: Report a lack of matches when calling `notmuch-show'.
  2016-02-10 19:52     ` David Edmondson
@ 2016-02-10 20:14       ` David Edmondson
  2016-02-10 21:05       ` Mark Walters
  1 sibling, 0 replies; 6+ messages in thread
From: David Edmondson @ 2016-02-10 20:14 UTC (permalink / raw)
  To: Mark Walters, notmuch

On Wed, Feb 10 2016, David Edmondson wrote:
>> Two minor things, one tiny nit below; and I wonder whether just having
>> the buffer say "No search results" (or something similar) and leave
>> the user to kill it would be nicer than dinging (and more in line with
>> the way search and tree behave).
>>
>> [In some sense I think this way is right and search and tree are wrong,
>> but that is probably difficult to get round as search and tree run
>> asynchronously.]
>
> What if we did "notmuch count $query" first in the search and tree case,
> and did the "(ding) (message ...)" thing if the count returned 0? (Just
> wondering about whether having everything behave that way would be
> possible and acceptable.)

This is pretty easy for `notmuch-search', and "notmuch count $query"
seems very quick. The changes are a bit more involved for `notmuch-tree'.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] emacs: Report a lack of matches when calling `notmuch-show'.
  2016-02-10 19:52     ` David Edmondson
  2016-02-10 20:14       ` David Edmondson
@ 2016-02-10 21:05       ` Mark Walters
  1 sibling, 0 replies; 6+ messages in thread
From: Mark Walters @ 2016-02-10 21:05 UTC (permalink / raw)
  To: David Edmondson, notmuch


On Wed, 10 Feb 2016, David Edmondson <dme@dme.org> wrote:
> On Wed, Feb 10 2016, Mark Walters wrote:
>> This basically looks fine to me and all tests pass. The code movement
>> and cleanup all looks fine.
>
> Thanks.
>
>> Two minor things, one tiny nit below; and I wonder whether just having
>> the buffer say "No search results" (or something similar) and leave
>> the user to kill it would be nicer than dinging (and more in line with
>> the way search and tree behave).
>>
>> [In some sense I think this way is right and search and tree are wrong,
>> but that is probably difficult to get round as search and tree run
>> asynchronously.]
>
> What if we did "notmuch count $query" first in the search and tree case,
> and did the "(ding) (message ...)" thing if the count returned 0? (Just
> wondering about whether having everything behave that way would be
> possible and acceptable.)
>
> The original impetus for this change was someone who hits an id: button
> that is either a false match (i.e it wasn't ever intended to be a
> notmuch reference) or for a message that they don't have. In both of
> those cases popping up a buffer that says only "No match." would be
> annoying. If we were considering the case where people are using "M-x
> notmuch-show", it seems less clear on the right thing to do, but overall
> I prefer this approach to the useless buffer that I have to kill/quit.
>

I am quite happy to leave it as you have it with the ding; currently we
have an obscure error message and this is obviously better so (with the
comment change below) +1 from me.

Best wishes

Mark

>>> +      ;; Cache the original tags for each message so that we can display
>>> +      ;; changes.
>>
>> ^^ I think "Store the original tags for each message" would be better,
>> particularly as this is nothing to do with the tag cache as used by say
>> notmuch-tag-clear-cache.
>
> Agreed - fixed.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-02-10 21:05 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-10 14:01 [PATCH v2] (notmuch-show "id:doesnotexist") should not throw an error David Edmondson
2016-02-10 14:01 ` [PATCH v2] emacs: Report a lack of matches when calling `notmuch-show' David Edmondson
2016-02-10 18:50   ` Mark Walters
2016-02-10 19:52     ` David Edmondson
2016-02-10 20:14       ` David Edmondson
2016-02-10 21:05       ` Mark Walters

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).