Re: Bug: ol-notmuch.el: calls `notmuch-show' with arbitrary search query
Hello David, On Sun 01 Dec 2019 at 09:01PM +00, David Edmondson wrote: > On Friday, 2019-11-29 at 13:01:00 -07, Sean Whitton wrote: > >> On Wed 27 Nov 2019 at 08:52PM +00, David Edmondson wrote: >>> It's a lot to ask, I know, but if you could provide a specific set of >>> messages with a corresponding initial query that fails for you after >>> limiting using your function, it would be helpful. >> >> You should be able to download an mbox of the thread I was using for >> testing here: >> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=945195;mbox=yes >> >> The Org-mode link I was using to open the thread is >> [[notmuch:id:20191125214551.31596-1-...@fifthhorseman.net][dkg's v2 for >> S/MIME decryption]] > > This works exactly as I would expect with the patch I previously sent. > > I downloaded the messages, added them to my corpus and remove the inbox > and unread tags. > > Then, if I execute: > > (notmuch-show “thread:{id:20191125214551.31596-1-...@fifthhorseman.net}”) > > and use your function the list of messages open is limited as I > expected. > > “As I expected” means that if I pass no prefix, all of the messages that > include patches are open. With a prefix argument of “1”, only the first > set of patches are open. With a prefix argument of “2”, only the v2 > patches are open. > > Did I misunderstand the behaviour that you expect? No, that's what's meant to happen, so I guess that my steps to reproduce were not accurate. As I mentioned, I don't want to spend a lot of time on this since I think the bug is in ol-notmuch.el. Regardless, thank you for your interest. (You may be interested in the latest version of mailscripts which contains a rewritten notmuch-extract-patch with new features.) -- Sean Whitton ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: Bug: ol-notmuch.el: calls `notmuch-show' with arbitrary search query
On Friday, 2019-11-29 at 13:01:00 -07, Sean Whitton wrote: > On Wed 27 Nov 2019 at 08:52PM +00, David Edmondson wrote: >> It's a lot to ask, I know, but if you could provide a specific set of >> messages with a corresponding initial query that fails for you after >> limiting using your function, it would be helpful. > > You should be able to download an mbox of the thread I was using for > testing here: > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=945195;mbox=yes > > The Org-mode link I was using to open the thread is > [[notmuch:id:20191125214551.31596-1-...@fifthhorseman.net][dkg's v2 for > S/MIME decryption]] This works exactly as I would expect with the patch I previously sent. I downloaded the messages, added them to my corpus and remove the inbox and unread tags. Then, if I execute: (notmuch-show “thread:{id:20191125214551.31596-1-...@fifthhorseman.net}”) and use your function the list of messages open is limited as I expected. “As I expected” means that if I pass no prefix, all of the messages that include patches are open. With a prefix argument of “1”, only the first set of patches are open. With a prefix argument of “2”, only the v2 patches are open. Did I misunderstand the behaviour that you expect? > Please excuse me for not preparing more formal steps to reproduce -- > since I consider the bug to be in ol-notmuch.el and my fix to that file > prevents this bug from showing up, I am not really willing to invest a > lot of time in preparing a different sort of fix. Understood. My interest is in improving notmuch generally. dme. -- No proper time of day. ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: Bug: ol-notmuch.el: calls `notmuch-show' with arbitrary search query
Dear David, Thank you again for your interest. On Wed 27 Nov 2019 at 08:52PM +00, David Edmondson wrote: > If this does what I think, which is to filter the current thread to show > only those messages that contain actual patches (with an optional > version number), then, with the previous patch I sent, it seems to work > just fine for me if the original `notmuch-show' query is something like > “thread:a or thread:b”, where those threads both contain both patches > and comments on the patches. > > An example call I used initially is: > > (notmuch-show "thread:{id:20191117222929.1064-1-j...@jb55.com} or > thread:{id:20190419035659.28961-1-congdan...@gmail.com}") > > Hopefully this is a query that will work for you as well. Limiting this > using your function behaves as I would expect. I don't have those messages, I'm afraid. > It's a lot to ask, I know, but if you could provide a specific set of > messages with a corresponding initial query that fails for you after > limiting using your function, it would be helpful. You should be able to download an mbox of the thread I was using for testing here: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=945195;mbox=yes The Org-mode link I was using to open the thread is [[notmuch:id:20191125214551.31596-1-...@fifthhorseman.net][dkg's v2 for S/MIME decryption]] Please excuse me for not preparing more formal steps to reproduce -- since I consider the bug to be in ol-notmuch.el and my fix to that file prevents this bug from showing up, I am not really willing to invest a lot of time in preparing a different sort of fix. 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. >> >> To my mind, it makes sense only to the extent that committing hacky >> workarounds is something which makes sense :) > > Well, if we stick to the idea that the first argument to `notmuch-show' > can only be a thread ID, then yes. > > Given that thread IDs have some annoying properties, it would be > convenient to allow the caller to pass an arbitrary query. Yes, I certainly agree that it would be a more useful API. -- Sean Whitton ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: Bug: ol-notmuch.el: calls `notmuch-show' with arbitrary search query
On Wednesday, 2019-11-27 at 10:42:59 -07, Sean Whitton wrote: > On Wed 27 Nov 2019 at 01:08PM +00, David Edmondson wrote: > >> 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.) > > I have this function which calls notmuch-show-filter-thread: > > (defun spw--notmuch-show-filter-thread-patches (&optional reroll-count) > (interactive "P") > (let ((subject-filter > (if reroll-count > (let ((n (prefix-numeric-value reroll-count))) >(if (= n 1) >"(subject:/\\[.*PATCH[^v]*\\]/ or > subject:/\\[.*PATCH.*v1.*\\]/)" > (concat "subject:/\\[.*PATCH.*v" (number-to-string n) > ".*\\]/"))) >"subject:/\\[.*PATCH.*\\]/ "))) > (notmuch-show-filter-thread > (concat "tag:unread or tag:flagged or (" > subject-filter > " and not subject:'Re:' and not subject:'Info received')" > > Copy-pasting the whole function since you mentioned you were interested > in the notmuch patch review functionality I'm working on, but let me > know if you want a narrower test case. If this does what I think, which is to filter the current thread to show only those messages that contain actual patches (with an optional version number), then, with the previous patch I sent, it seems to work just fine for me if the original `notmuch-show' query is something like “thread:a or thread:b”, where those threads both contain both patches and comments on the patches. An example call I used initially is: (notmuch-show "thread:{id:20191117222929.1064-1-j...@jb55.com} or thread:{id:20190419035659.28961-1-congdan...@gmail.com}") Hopefully this is a query that will work for you as well. Limiting this using your function behaves as I would expect. It's a lot to ask, I know, but if you could provide a specific set of messages with a corresponding initial query that fails for you after limiting using your function, it would be helpful. >>> 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. > > To my mind, it makes sense only to the extent that committing hacky > workarounds is something which makes sense :) Well, if we stick to the idea that the first argument to `notmuch-show' can only be a thread ID, then yes. Given that thread IDs have some annoying properties, it would be convenient to allow the caller to pass an arbitrary query. If we fix anything that is broken by that, I'm happy to provide a patch that changes the code (and any implicit or explicit documentation) accordingly. dme. -- This is the time. This is the record of the time. ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: Bug: ol-notmuch.el: calls `notmuch-show' with arbitrary search query
[dropping Org-mode list per David's suggestion] Hello, On Wed 27 Nov 2019 at 01:08PM +00, David Edmondson wrote: > 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.) I have this function which calls notmuch-show-filter-thread: (defun spw--notmuch-show-filter-thread-patches (&optional reroll-count) (interactive "P") (let ((subject-filter (if reroll-count (let ((n (prefix-numeric-value reroll-count))) (if (= n 1) "(subject:/\\[.*PATCH[^v]*\\]/ or subject:/\\[.*PATCH.*v1.*\\]/)" (concat "subject:/\\[.*PATCH.*v" (number-to-string n) ".*\\]/"))) "subject:/\\[.*PATCH.*\\]/ "))) (notmuch-show-filter-thread (concat "tag:unread or tag:flagged or (" subject-filter " and not subject:'Re:' and not subject:'Info received')" Copy-pasting the whole function since you mentioned you were interested in the notmuch patch review functionality I'm working on, but let me know if you want a narrower test case. >> 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. To my mind, it makes sense only to the extent that committing hacky workarounds is something which makes sense :) > 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)? Indeed. -- Sean Whitton ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: Bug: ol-notmuch.el: calls `notmuch-show' with arbitrary search query
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. ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: Bug: ol-notmuch.el: calls `notmuch-show' with arbitrary search query
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 ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: Bug: ol-notmuch.el: calls `notmuch-show' with arbitrary search query
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? 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) dme. -- I can't explain, you would not understand. This is not how I am. ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: Bug: ol-notmuch.el: calls `notmuch-show' with arbitrary search query
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 ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: Bug: ol-notmuch.el: calls `notmuch-show' with arbitrary search query
Hello, On Tue 26 Nov 2019 at 08:17PM +01, ra...@free.fr 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:f...@bar.baz", 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 ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: Bug: ol-notmuch.el: calls `notmuch-show' with arbitrary search query
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. ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: Bug: ol-notmuch.el: calls `notmuch-show' with arbitrary search query
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 ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: Bug: ol-notmuch.el: calls `notmuch-show' with arbitrary search query
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 ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Bug: ol-notmuch.el: calls `notmuch-show' with arbitrary search query
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 ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch