[PATCH 0/1] emacs: Make notmuch-show-next-thread return nil on failure

[PATCH 0/1] emacs: Make notmuch-show-next-thread return nil on failure

[Leo]

From: Leo Okawa Ericson <git@relevant-information.com>

Hello,

First some context on why I personally want this change.  I am
developing a package that allows users to interact with notmuch in a
specific way.  In particular, I need to know when
notmuch-show-next-thread succeeds or fails (there are no more messages
for this query).  See [1] for how I'm currently using notmuch.

I've implemented that change in this patch.  I've tested it manually,
but didn't understand how to make the test I wrote to work properly, so
I haven't included that in the patch.

Here is a basic test I tried to write in test/T450-emacs-show.sh, but
the first "assert" fails.

> test_begin_subtest "notmuch-search-show-thread returns non-nil on success"
> test_emacs_expect_t  '(notmuch-search "id:20091117203301.GV3165@dottiness.seas.harvard.edu")
> 	(unless (notmuch-search-show-thread)
>           (error "Expected non-nil when successfully showing a thread"))
> 	(when (notmuch-show-next-thread)
>         (error "Expected nil when there are no more threads"))
> t'


[1] https://git.sr.ht/~zetagon/el-secretario/tree/ab0572477ab2bdb2364256dd884555b052705659/item/el-secretario-notmuch.el#L109


Leo Okawa Ericson (1):
  emacs: Make notmuch-show-next-thread return nil on failure

 emacs/notmuch.el | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)


base-commit: e3ad0087f3453c89871acac8b11da8bab1ac54df
-- 
2.36.0

[PATCH 1/1] emacs: Make notmuch-show-next-thread return nil on failure

[Leo]

From: Leo Okawa Ericson <git@relevant-information.com>

Having notmuch-show-next-thread return non-nil on success and nil on
failure makes it easier for users to interact with notmuch via elisp.
---
 emacs/notmuch.el | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index c9cf80dc..a6198f4e 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -525,14 +525,15 @@ (defun notmuch-search-show-thread (&optional elide-toggle)
   (let ((thread-id (notmuch-search-find-thread-id)))
     (if thread-id
 	(notmuch-show thread-id
-		      elide-toggle
-		      (current-buffer)
-		      notmuch-search-query-string
-		      ;; Name the buffer based on the subject.
-		      (format "*%s*" (truncate-string-to-width
-				      (notmuch-search-find-subject)
-				      30 nil nil t)))
-      (message "End of search results."))))
+	       elide-toggle
+	       (current-buffer)
+	       notmuch-search-query-string
+	       ;; Name the buffer based on the subject.
+	       (format "*%s*" (truncate-string-to-width
+			       (notmuch-search-find-subject)
+			       30 nil nil t)))
+      (message "End of search results.")
+      nil)))
 
 (defun notmuch-tree-from-search-current-query ()
   "Tree view of current query."
-- 
2.36.0

Re: [PATCH 1/1] emacs: Make notmuch-show-next-thread return nil on failure

[Tomi Ollila]

On Thu, Apr 28 2022, Leo wrote:

> From: Leo Okawa Ericson <git@relevant-information.com>
>
> Having notmuch-show-next-thread return non-nil on success and nil on
> failure makes it easier for users to interact with notmuch via elisp.
> ---
>  emacs/notmuch.el | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index c9cf80dc..a6198f4e 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -525,14 +525,15 @@ (defun notmuch-search-show-thread (&optional elide-toggle)
>    (let ((thread-id (notmuch-search-find-thread-id)))
>      (if thread-id
>  	(notmuch-show thread-id
> -		      elide-toggle
> -		      (current-buffer)
> -		      notmuch-search-query-string
> -		      ;; Name the buffer based on the subject.
> -		      (format "*%s*" (truncate-string-to-width
> -				      (notmuch-search-find-subject)
> -				      30 nil nil t)))
> -      (message "End of search results."))))
> +	       elide-toggle

I don't see why this indentation change is happening...

> +	       (current-buffer)
> +	       notmuch-search-query-string
> +	       ;; Name the buffer based on the subject.
> +	       (format "*%s*" (truncate-string-to-width
> +			       (notmuch-search-find-subject)
> +			       30 nil nil t)))
> +      (message "End of search results.")
> +      nil)))
>  
>  (defun notmuch-tree-from-search-current-query ()
>    "Tree view of current query."
> -- 
> 2.36.0
>
> _______________________________________________
> notmuch mailing list -- notmuch@notmuchmail.org
> To unsubscribe send an email to notmuch-leave@notmuchmail.org

Re: [PATCH 1/1] emacs: Make notmuch-show-next-thread return nil on failure

[Leo Okawa Ericson]

Tomi Ollila <tomi.ollila@iki.fi> writes:

>
> I don't see why this indentation change is happening...
>

I thought the indentation was inconsistent with the rest of the file.  I
tried running the auto-indenter on a other more complex functions which
changed nothing, but it did change it on this function so I assumed it
was good to include it.  If it's bad etiquette, wrong or unnecessary I'm happy
to amend the patch.

/Leo

Re: [PATCH 1/1] emacs: Make notmuch-show-next-thread return nil on failure

[David Bremner]

Leo Okawa Ericson <git@relevant-information.com> writes:

> Tomi Ollila <tomi.ollila@iki.fi> writes:
>
>>
>> I don't see why this indentation change is happening...
>>
>
> I thought the indentation was inconsistent with the rest of the file.  I
> tried running the auto-indenter on a other more complex functions which
> changed nothing, but it did change it on this function so I assumed it
> was good to include it.  If it's bad etiquette, wrong or unnecessary I'm happy
> to amend the patch.

In general indentation (and whitespace) cleanup is fine, but should be
seperate commits labelled as such.

d

Re: [PATCH 1/1] emacs: Make notmuch-show-next-thread return nil on failure

[David Bremner]

Leo <git@relevant-information.com> writes:

> From: Leo Okawa Ericson <git@relevant-information.com>
>
> Having notmuch-show-next-thread return non-nil on success and nil on
> failure makes it easier for users to interact with notmuch via elisp.

You talk about notmuch-show-next-thread, but you seem to be modifying
notmuch-search-show-thread. That actually seems like a bit of a strange
function to invoke programatically, since it deduces the argument from
the current buffer.  It might be better for your code to call
notmuch-show directly, since notmuch-search-show-thread is pretty tied
to the internals of notmuch-search-mode.

In any case:

If you change the API to have a meaningful return value, please update
the docstring for the function

We generally need a test for any new feature, and this is arguably a new
feature. If you need help constructing a test, feel free to ask. There
are several examples in  test/T*emacs*.sh.

Re: [PATCH 1/1] emacs: Make notmuch-show-next-thread return nil on failure

[Leo Okawa Ericson]

David Bremner <david@tethera.net> writes:

> Leo <git@relevant-information.com> writes:
>
>> From: Leo Okawa Ericson <git@relevant-information.com>
>>
>> Having notmuch-show-next-thread return non-nil on success and nil on
>> failure makes it easier for users to interact with notmuch via elisp.
>
> You talk about notmuch-show-next-thread, but you seem to be modifying
> notmuch-search-show-thread. That actually seems like a bit of a strange
> function to invoke programatically, since it deduces the argument from
> the current buffer.  It might be better for your code to call
> notmuch-show directly, since notmuch-search-show-thread is pretty tied
> to the internals of notmuch-search-mode.
>

I mentioned this in the reroll of the patch, but I modified
notmuch-search-show-thread because the return value of
notmuch-show-next-thread depends on it.  In case this is an XY
problem[1], what I want is the ability to go through each email in the
search buffer (like notmuch-show-next-thread does), and when there are
no more messages left I want to call a function.  Changing
notmuch-search-show-thread (and thereby notmuch-show-next-thread),
seemed to be the least intrusive way to do it.

[1] https://en.wikipedia.org/wiki/XY_problem

/Leo