Bug: ol-notmuch.el: calls `notmuch-show' with arbitrary search query

Bug: ol-notmuch.el: calls `notmuch-show' with arbitrary search query

[Sean Whitton]

Dear maintainers,

The function `org-notmuch-follow-link' in {org,ol}-notmuch.el calls
`notmuch-show' with an arbitrary notmuch search query.  However, the
docstring for `notmuch-show' specifies that a notmuch thread ID, rather
than an arbitrary notmuch query, should be supplied to `notmuch-show'.

The effect of this is that the variable `notmuch-show-thread-id' may
contain an arbitrary search query rather than a thread ID.  That broke
some code of mine which uses that variable.

`org-notmuch-follow-link' needs to continue to accept an arbitrary query
(as notmuch thread IDs are not stable), but it should convert it to a
thread ID before passing it to `notmuch-show'.

-- 
Sean Whitton

Re: Bug: ol-notmuch.el: calls `notmuch-show' with arbitrary search query

[Sean Whitton]

Dear maintainers,

On Thu 21 Nov 2019 at 02:37PM -07, Sean Whitton wrote:

> The function `org-notmuch-follow-link' in {org,ol}-notmuch.el calls
> `notmuch-show' with an arbitrary notmuch search query.  However, the
> docstring for `notmuch-show' specifies that a notmuch thread ID, rather
> than an arbitrary notmuch query, should be supplied to `notmuch-show'.
>
> The effect of this is that the variable `notmuch-show-thread-id' may
> contain an arbitrary search query rather than a thread ID.  That broke
> some code of mine which uses that variable.
>
> `org-notmuch-follow-link' needs to continue to accept an arbitrary query
> (as notmuch thread IDs are not stable), but it should convert it to a
> thread ID before passing it to `notmuch-show'.

Here is my workaround.  If this approach seems sensible I can prepare a
patch to `org-notmuch-follow-link` in ol-notmuch.el.

    (use-package org-notmuch
      :init
      ;; the default value for `org-notmuch-open-function' is
      ;; `org-notmuch-follow-link', but that function is broken: it calls
      ;; `notmuch-show' with a search query rather than a thread ID.  This
      ;; causes `notmuch-show-thread-id' to be populated with a value
      ;; which is not a thread ID, which breaks various other things
      ;;
      ;; so use a custom function instead
      (defun spw--org-notmuch-follow-link (search)
        (let ((thread-id
               (substring
                (shell-command-to-string
                 (combine-and-quote-strings (list "notmuch" "search"
                                                  "--output=threads" "--limit=1"
                                                  "--format=text"
                                                  "--format-version=4" search)))
                0 -1)))
          (notmuch-show thread-id nil nil search search)))
      (setq org-notmuch-open-function 'spw--org-notmuch-follow-link))

-- 
Sean Whitton

Re: Bug: ol-notmuch.el: calls `notmuch-show' with arbitrary search query

[racin]

Hi,


> 
> Here is my workaround.  If this approach seems sensible I can prepare a
> patch to `org-notmuch-follow-link` in ol-notmuch.el.

Your approach probably works most of the time, but I don't like the idea 
of having to perform 2 queries when one should be enough.

I think a better approach would be to change notmuch-show (or add a new version) that would
allow taking arbitrary queries (and especially, message ids) as input. This probably used to be the case.

Note that the command line notmuch show function does accept arbitrary search terms as an argument

I think updating notmuch-show to compute the thread id from the query (instead of assuming that the input is
a thread id) would be the best way to go, but you should see that with the notmuch developers.

Best regards
Matthieu Lemerre


> 
>    (use-package org-notmuch
>      :init
>      ;; the default value for `org-notmuch-open-function' is
>      ;; `org-notmuch-follow-link', but that function is broken: it calls
>      ;; `notmuch-show' with a search query rather than a thread ID.  This
>      ;; causes `notmuch-show-thread-id' to be populated with a value
>      ;; which is not a thread ID, which breaks various other things
>      ;;
>      ;; so use a custom function instead
>      (defun spw--org-notmuch-follow-link (search)
>        (let ((thread-id
>               (substring
>                (shell-command-to-string
>                 (combine-and-quote-strings (list "notmuch" "search"
>                                                  "--output=threads" "--limit=1"
>                                                  "--format=text"
>                                                  "--format-version=4" search)))
>                0 -1)))
>          (notmuch-show thread-id nil nil search search)))
>      (setq org-notmuch-open-function 'spw--org-notmuch-follow-link))
> 
> --
> Sean Whitton

Re: Bug: ol-notmuch.el: calls `notmuch-show' with arbitrary search query

[David Edmondson]

On Thursday, 2019-11-21 at 14:37:44 -07, Sean Whitton wrote:

> The function `org-notmuch-follow-link' in {org,ol}-notmuch.el calls
> `notmuch-show' with an arbitrary notmuch search query.  However, the
> docstring for `notmuch-show' specifies that a notmuch thread ID, rather
> than an arbitrary notmuch query, should be supplied to `notmuch-show'.
>
> The effect of this is that the variable `notmuch-show-thread-id' may
> contain an arbitrary search query rather than a thread ID.  That broke
> some code of mine which uses that variable.

Could you explain how you were using `notmuch-show-thread-id' in a way
that was broken by the presence of an arbitrary query?

(Not arguing right or wrong, just trying to understand the problem.)

> `org-notmuch-follow-link' needs to continue to accept an arbitrary query
> (as notmuch thread IDs are not stable), but it should convert it to a
> thread ID before passing it to `notmuch-show'.

dme.
-- 
I'd rather be with you than flying through space.

Re: Bug: ol-notmuch.el: calls `notmuch-show' with arbitrary search query

[Sean Whitton]

Hello,

On Tue 26 Nov 2019 at 08:17PM +01, racin-GANU6spQydw@public.gmane.org wrote:

> Your approach probably works most of the time, but I don't like the idea
> of having to perform 2 queries when one should be enough.
>
> I think a better approach would be to change notmuch-show (or add a
> new version) that would allow taking arbitrary queries (and
> especially, message ids) as input. This probably used to be the case.
>
> Note that the command line notmuch show function does accept arbitrary
> search terms as an argument
>
> I think updating notmuch-show to compute the thread id from the query
> (instead of assuming that the input is a thread id) would be the best
> way to go, but you should see that with the notmuch developers.

I agree with you that it would be best if `notmuch-show' were extended
to accept arbitrary notmuch queries instead of only thread IDs.

However, I also believe that ol-notmuch.el should be updated in the
meantime.  notmuch.el presently exposes an API, `notmuch-show', which
takes a thread ID.  ol-notmuch.el is misusing that API by passing in a
value of a different type -- a notmuch query rather than a thread ID.

That's a bug: even if an API is more limited than we would like it to
be, we should use it in the way that it is documented to be used by its
developers.

This misuse of the API causes numerous other hard-to-diagnose bugs.  For
example, if you use `org-notmuch-follow-link' to open an Org-mode link
like "notmuch:id:foo-o+HKXD+gRuM@public.gmane.org", the standard notmuch command
`notmuch-show-filter-thread' doesn't work.  And if you use `C-u c i` to
yank the thread ID for pasting into a shell, say, you will yank a value
which is not a thread ID.

-- 
Sean Whitton

Re: Bug: ol-notmuch.el: calls `notmuch-show' with arbitrary search query

[Sean Whitton]

Hello David,

On Tue 26 Nov 2019 at 08:05PM +00, David Edmondson wrote:

> Could you explain how you were using `notmuch-show-thread-id' in a way
> that was broken by the presence of an arbitrary query?

I've observed that the standard notmuch command
`notmuch-show-filter-thread' doesn't work in a buffer opened by
`org-notmuch-follow-link'.

Further, my package 'mailscripts' tries to pass the current value of
`notmuch-show-thread-id' to notmuch-extract-patch(1).

    https://git.spwhitton.name/mailscripts/tree/mailscripts.el#n72

    https://manpages.debian.org/notmuch-extract-patch

If `notmuch-show-thread-id' contains a query which returns a single
message, the wrong value is passed to notmuch-extract-patch(1), such
that it may not extract all of the patches in the thread.

-- 
Sean Whitton

Re: Bug: ol-notmuch.el: calls `notmuch-show' with arbitrary search query

[David Edmondson]

[-- Attachment #1: Type: text/plain, Size: 1330 bytes --]

On Tuesday, 2019-11-26 at 14:57:28 -07, Sean Whitton wrote:

> On Tue 26 Nov 2019 at 08:05PM +00, David Edmondson wrote:
>
>> Could you explain how you were using `notmuch-show-thread-id' in a way
>> that was broken by the presence of an arbitrary query?
>
> I've observed that the standard notmuch command
> `notmuch-show-filter-thread' doesn't work in a buffer opened by
> `org-notmuch-follow-link'.

The poor behaviour is just a side effect of the way that queries are
composed to implement the filter functionality. Does the attached patch
help?

> Further, my package 'mailscripts' tries to pass the current value of
> `notmuch-show-thread-id' to notmuch-extract-patch(1).
>
>     https://git.spwhitton.name/mailscripts/tree/mailscripts.el#n72
>
>     https://manpages.debian.org/notmuch-extract-patch
>
> If `notmuch-show-thread-id' contains a query which returns a single
> message, the wrong value is passed to notmuch-extract-patch(1), such
> that it may not extract all of the patches in the thread.

It's not clear to me that this is broken.

notmuch-extract-patch seems to be properly extracting patches from the
messages that match the query.

If the current `notmuch-show' buffer query doesn't match the entire
thread, why should `notmuch-extract-thread-patches' be expected to apply
patches from the whole thread?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: query-composition.patch --]
[-- Type: text/x-patch, Size: 537 bytes --]

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index e13ca3d7..ecbc03d2 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1311,7 +1311,7 @@ and THREAD.  The next query is THREAD alone, and serves as a
 fallback if the prior matches no messages."
   (let (queries)
     (push (list thread) queries)
-    (if context (push (list thread "and (" context ")") queries))
+    (if context (push (list "(" thread ") and (" context ")") queries))
     queries))
 
 (defun notmuch-show--build-buffer (&optional state)

[-- Attachment #3: Type: text/plain, Size: 75 bytes --]


dme.
-- 
I can't explain, you would not understand. This is not how I am.

[-- Attachment #4: Type: text/plain, Size: 0 bytes --]

Re: Bug: ol-notmuch.el: calls `notmuch-show' with arbitrary search query

[Sean Whitton]

Hello David,

Thank you for the investigation.

On Tue 26 Nov 2019 at 10:52PM +00, David Edmondson wrote:

> The poor behaviour is just a side effect of the way that queries are
> composed to implement the filter functionality. Does the attached patch
> help?

Unfortunately, it is still broken in my test case.

>> Further, my package 'mailscripts' tries to pass the current value of
>> `notmuch-show-thread-id' to notmuch-extract-patch(1).
>>
>>     https://git.spwhitton.name/mailscripts/tree/mailscripts.el#n72
>>
>>     https://manpages.debian.org/notmuch-extract-patch
>>
>> If `notmuch-show-thread-id' contains a query which returns a single
>> message, the wrong value is passed to notmuch-extract-patch(1), such
>> that it may not extract all of the patches in the thread.
>
> It's not clear to me that this is broken.
>
> notmuch-extract-patch seems to be properly extracting patches from the
> messages that match the query.
>
> If the current `notmuch-show' buffer query doesn't match the entire
> thread, why should `notmuch-extract-thread-patches' be expected to apply
> patches from the whole thread?

The purpose of `notmuch-extract-thread-patches' is to extract a whole
git-send-email(1) patch series at a time, because that is usually what
one wants to do.  There are `notmuch-extract-message-patches' and
`notmuch-show-pipe-message' for single patches.

(I note that this is a mailscripts design question, not strictly
relevant to the issue of ol-notmuch.el causing the
notmuch-show-thread-id variable to be mispopulated.  Thank you for your
engagement with mailscripts, regardless!)

-- 
Sean Whitton

Re: Bug: ol-notmuch.el: calls `notmuch-show' with arbitrary search query

[David Edmondson]

On Tuesday, 2019-11-26 at 16:25:29 -07, Sean Whitton wrote:

> On Tue 26 Nov 2019 at 10:52PM +00, David Edmondson wrote:
>
>> The poor behaviour is just a side effect of the way that queries are
>> composed to implement the filter functionality. Does the attached patch
>> help?
>
> Unfortunately, it is still broken in my test case.

Could you describe your test case please? (We could remove emacs-orgmode
for that conversation if you think it appropriate.)

>>> Further, my package 'mailscripts' tries to pass the current value of
>>> `notmuch-show-thread-id' to notmuch-extract-patch(1).
>>>
>>>     https://git.spwhitton.name/mailscripts/tree/mailscripts.el#n72
>>>
>>>     https://manpages.debian.org/notmuch-extract-patch
>>>
>>> If `notmuch-show-thread-id' contains a query which returns a single
>>> message, the wrong value is passed to notmuch-extract-patch(1), such
>>> that it may not extract all of the patches in the thread.
>>
>> It's not clear to me that this is broken.
>>
>> notmuch-extract-patch seems to be properly extracting patches from the
>> messages that match the query.
>>
>> If the current `notmuch-show' buffer query doesn't match the entire
>> thread, why should `notmuch-extract-thread-patches' be expected to apply
>> patches from the whole thread?
>
> The purpose of `notmuch-extract-thread-patches' is to extract a whole
> git-send-email(1) patch series at a time, because that is usually what
> one wants to do.  There are `notmuch-extract-message-patches' and
> `notmuch-show-pipe-message' for single patches.

Then I think the approach that you have (wrapping the query in
thread:{}) makes perfect sense.

> (I note that this is a mailscripts design question, not strictly
> relevant to the issue of ol-notmuch.el causing the
> notmuch-show-thread-id variable to be mispopulated.  Thank you for your
> engagement with mailscripts, regardless!)

Well, I'm interested in the functionality that it provides :-)

Presumably this all arises because thread IDs are neither stable nor
portable, so they are inappropriate to store in an external document (as
an org-mode link in this case)?

dme.
-- 
Hello? Is anybody home? Well, you don't know me, but I know you.