Gnus development mailing list
 help / color / mirror / Atom feed
* Help testing gnus-search fixups
@ 2021-06-28 16:42 Eric Abrahamsen
  2021-06-28 18:00 ` Adam Sjøgren
  2021-06-29 11:27 ` Eric S Fraga
  0 siblings, 2 replies; 15+ messages in thread
From: Eric Abrahamsen @ 2021-06-28 16:42 UTC (permalink / raw)
  To: ding

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

Hi all,

I've reworked the way gnus-search parses the output from search
utilities like notmuch, which should eliminate some of the bugs and
funny behavior that's been reported here. I'd be much happier if I had
some more eyes on it -- if you're using notmuch or namazu, please help
me give this a whirl!

Thanks,
Eric


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Rework-gnus-search-indexed-parse-output.patch --]
[-- Type: text/x-patch, Size: 5012 bytes --]

From 7f9e962b890bfa0924ca17a7abd8cc83e1b8d2bf Mon Sep 17 00:00:00 2001
From: Eric Abrahamsen <eric@ericabrahamsen.net>
Date: Sat, 26 Jun 2021 10:16:19 -0700
Subject: [PATCH 1/1] Rework gnus-search-indexed-parse-output

* lisp/gnus/gnus-search.el (gnus-search-indexed-parse-output): Be more
careful about matching filesystem paths to Gnus group names; make
absolutely sure that we only return valid article numbers.
---
 lisp/gnus/gnus-search.el | 94 ++++++++++++++++++----------------------
 1 file changed, 42 insertions(+), 52 deletions(-)

diff --git a/lisp/gnus/gnus-search.el b/lisp/gnus/gnus-search.el
index 70bde264c1..add34e7e02 100644
--- a/lisp/gnus/gnus-search.el
+++ b/lisp/gnus/gnus-search.el
@@ -1351,68 +1351,58 @@ gnus-search-run-search
 
 (cl-defmethod gnus-search-indexed-parse-output ((engine gnus-search-indexed)
 						server query &optional groups)
-  (let ((prefix (slot-value engine 'remove-prefix))
-	(group-regexp (when groups
-			(mapconcat
-			 (lambda (group-name)
-			   (mapconcat #'regexp-quote
-				      (split-string
-				       (gnus-group-real-name group-name)
-				       "[.\\/]")
-				      "[.\\\\/]"))
-			 groups
-			 "\\|")))
-	artlist vectors article group)
+  (let ((prefix (or (slot-value engine 'remove-prefix)
+                    ""))
+	artlist article group)
     (goto-char (point-min))
+    ;; Prep prefix, we want to at least be removing the root
+    ;; filesystem separator.
+    (when (stringp prefix)
+      (setq prefix (file-name-as-directory
+                    (expand-file-name prefix "/"))))
     (while (not (or (eobp)
                     (looking-at-p
                      "\\(?:[[:space:]\n]+\\)?Process .+ finished")))
       (pcase-let ((`(,f-name ,score) (gnus-search-indexed-extract engine)))
 	(when (and f-name
                    (file-readable-p f-name)
-		   (null (file-directory-p f-name))
-		   (or (null groups)
-		       (and (gnus-search-single-p query)
-			    (alist-get 'thread query))
-		       (string-match-p group-regexp f-name)))
-	  (push (list f-name score) artlist))))
+		   (null (file-directory-p f-name)))
+          (setq group
+                (replace-regexp-in-string
+	         "[/\\]" "."
+	         (replace-regexp-in-string
+	          "/?\\(cur\\|new\\|tmp\\)?/\\'" ""
+	          (replace-regexp-in-string
+	           "\\`\\." ""
+	           (string-remove-prefix
+                    prefix (file-name-directory f-name))
+                   nil t)
+	          nil t)
+	         nil t))
+          (setq article (file-name-nondirectory f-name)
+                article
+                ;; TODO: Provide a cleaner way of producing final
+                ;; article numbers for the various backends.
+                (if (string-match-p "\\`[[:digit:]]+\\'" article)
+		    (string-to-number article)
+		  (nnmaildir-base-name-to-article-number
+		   (substring article 0 (string-match ":" article))
+		   group (string-remove-prefix "nnmaildir:" server))))
+          (when (and (numberp article)
+                     (or (null groups)
+                         (member group groups)))
+	    (push (list f-name article group score)
+                  artlist)))))
     ;; Are we running an additional grep query?
     (when-let ((grep-reg (alist-get 'grep query)))
       (setq artlist (gnus-search-grep-search engine artlist grep-reg)))
-    ;; Prep prefix.
-    (when (and prefix (null (string-empty-p prefix)))
-      (setq prefix (file-name-as-directory (expand-file-name prefix))))
-    ;; Turn (file-name score) into [group article score].
-    (pcase-dolist (`(,f-name ,score) artlist)
-      (setq article (file-name-nondirectory f-name)
-	    group (file-name-directory f-name))
-      ;; Remove prefix.
-      (when prefix
-	(setq group (string-remove-prefix prefix group)))
-      ;; Break the directory name down until it's something that
-      ;; (probably) can be used as a group name.
-      (setq group
-	    (replace-regexp-in-string
-	     "[/\\]" "."
-	     (replace-regexp-in-string
-	      "/?\\(cur\\|new\\|tmp\\)?/\\'" ""
-	      (replace-regexp-in-string
-	       "^[./\\]" ""
-	       group nil t)
-	      nil t)
-	     nil t))
-
-      (push (vector (gnus-group-full-name group server)
-		    (if (string-match-p "\\`[[:digit:]]+\\'" article)
-			(string-to-number article)
-		      (nnmaildir-base-name-to-article-number
-		       (substring article 0 (string-match ":" article))
-		       group (string-remove-prefix "nnmaildir:" server)))
-		    (if (numberp score)
-			score
-		      (string-to-number score)))
-	    vectors))
-    vectors))
+    ;; Munge into the list of vectors expected by nnselect.
+    (mapcar (pcase-lambda (`(,_ ,article ,group ,score))
+              (vector group article
+                      (if (numberp score)
+			  score
+		        (string-to-number score))))
+            artlist)))
 
 (cl-defmethod gnus-search-indexed-extract ((_engine gnus-search-indexed))
   "Base implementation treats the whole line as a filename, and
-- 
2.32.0


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

* Re: Help testing gnus-search fixups
  2021-06-28 16:42 Help testing gnus-search fixups Eric Abrahamsen
@ 2021-06-28 18:00 ` Adam Sjøgren
  2021-06-28 23:15   ` Eric Abrahamsen
  2021-06-29 11:27 ` Eric S Fraga
  1 sibling, 1 reply; 15+ messages in thread
From: Adam Sjøgren @ 2021-06-28 18:00 UTC (permalink / raw)
  To: ding

Eric writes:

> if you're using notmuch or namazu, please help me give this a whirl!

Which version of Emacs is your patch against? I just tried applying it
to master@2a33fc8d19 and got:

  checking file lisp/gnus/gnus-search.el
  Hunk #1 FAILED at 1351.
  1 out of 1 hunk FAILED


  Best regards,

    Adam

-- 
 "Synge? Hvorfor skal vi altid synge,                       Adam Sjøgren
  bare fordi det er en tegnefilm?"                     asjo@koldfront.dk



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

* Re: Help testing gnus-search fixups
  2021-06-28 18:00 ` Adam Sjøgren
@ 2021-06-28 23:15   ` Eric Abrahamsen
  2021-06-29  6:04     ` Thomas Alexander Gerds
  2021-06-29  9:30     ` Adam Sjøgren
  0 siblings, 2 replies; 15+ messages in thread
From: Eric Abrahamsen @ 2021-06-28 23:15 UTC (permalink / raw)
  To: Adam Sjøgren; +Cc: ding

Adam Sjøgren <asjo@koldfront.dk> writes:

> Eric writes:
>
>> if you're using notmuch or namazu, please help me give this a whirl!
>
> Which version of Emacs is your patch against? I just tried applying it
> to master@2a33fc8d19 and got:

It applies to up-to-date master -- there's one other change that went
into this function since your current revision. The changes are confined
to the one function, so you could also just eval this definition:

(cl-defmethod gnus-search-indexed-parse-output ((engine gnus-search-indexed)
						server query &optional groups)
  (let ((prefix (or (slot-value engine 'remove-prefix)
                    ""))
	artlist article group)
    (goto-char (point-min))
    ;; Prep prefix, we want to at least be removing the root
    ;; filesystem separator.
    (when (stringp prefix)
      (setq prefix (file-name-as-directory
                    (expand-file-name prefix "/"))))
    (while (not (or (eobp)
                    (looking-at-p
                     "\\(?:[[:space:]\n]+\\)?Process .+ finished")))
      (pcase-let ((`(,f-name ,score) (gnus-search-indexed-extract engine)))
	(when (and f-name
                   (file-readable-p f-name)
		   (null (file-directory-p f-name)))
          (setq group
                (replace-regexp-in-string
	         "[/\\]" "."
	         (replace-regexp-in-string
	          "/?\\(cur\\|new\\|tmp\\)?/\\'" ""
	          (replace-regexp-in-string
	           "\\`\\." ""
	           (string-remove-prefix
                    prefix (file-name-directory f-name))
                   nil t)
	          nil t)
	         nil t))
          (setq article (file-name-nondirectory f-name)
                article
                ;; TODO: Provide a cleaner way of producing final
                ;; article numbers for the various backends.
                (if (string-match-p "\\`[[:digit:]]+\\'" article)
		    (string-to-number article)
		  (nnmaildir-base-name-to-article-number
		   (substring article 0 (string-match ":" article))
		   group (string-remove-prefix "nnmaildir:" server))))
          (when (and (numberp article)
                     (or (null groups)
                         (member group groups)))
	    (push (list f-name article group score)
                  artlist)))))
    ;; Are we running an additional grep query?
    (when-let ((grep-reg (alist-get 'grep query)))
      (setq artlist (gnus-search-grep-search engine artlist grep-reg)))
    ;; Munge into the list of vectors expected by nnselect.
    (mapcar (pcase-lambda (`(,_ ,article ,group ,score))
              (vector group article
                      (if (numberp score)
			  score
		        (string-to-number score))))
            artlist)))


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

* Re: Help testing gnus-search fixups
  2021-06-28 23:15   ` Eric Abrahamsen
@ 2021-06-29  6:04     ` Thomas Alexander Gerds
  2021-06-29  9:30     ` Adam Sjøgren
  1 sibling, 0 replies; 15+ messages in thread
From: Thomas Alexander Gerds @ 2021-06-29  6:04 UTC (permalink / raw)
  To: Eric Abrahamsen; +Cc: Adam Sjøgren, ding


I see 2 issues:

1. you may need 

(setq group (gnus-group-full-name group server))

before

(setq article (file-name-nondirectory f-name) ...

2. with the patch 

 (member group groups)

is nil when I use search-group-spec to '("nnml:" . ("nnml:")) to match
all nnml groups.

cheers

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> Adam Sjøgren <asjo@koldfront.dk> writes:
>
>> Eric writes:
>>
>>> if you're using notmuch or namazu, please help me give this a whirl!
>>
>> Which version of Emacs is your patch against? I just tried applying it
>> to master@2a33fc8d19 and got:
>
> It applies to up-to-date master -- there's one other change that went
> into this function since your current revision. The changes are confined
> to the one function, so you could also just eval this definition:
>
> (cl-defmethod gnus-search-indexed-parse-output ((engine gnus-search-indexed)
> 						server query &optional groups)
>   (let ((prefix (or (slot-value engine 'remove-prefix)
>                     ""))
> 	artlist article group)
>     (goto-char (point-min))
>     ;; Prep prefix, we want to at least be removing the root
>     ;; filesystem separator.
>     (when (stringp prefix)
>       (setq prefix (file-name-as-directory
>                     (expand-file-name prefix "/"))))
>     (while (not (or (eobp)
>                     (looking-at-p
>                      "\\(?:[[:space:]\n]+\\)?Process .+ finished")))
>       (pcase-let ((`(,f-name ,score) (gnus-search-indexed-extract engine)))
> 	(when (and f-name
>                    (file-readable-p f-name)
> 		   (null (file-directory-p f-name)))
>           (setq group
>                 (replace-regexp-in-string
> 	         "[/\\]" "."
> 	         (replace-regexp-in-string
> 	          "/?\\(cur\\|new\\|tmp\\)?/\\'" ""
> 	          (replace-regexp-in-string
> 	           "\\`\\." ""
> 	           (string-remove-prefix
>                     prefix (file-name-directory f-name))
>                    nil t)
> 	          nil t)
> 	         nil t))
>           (setq article (file-name-nondirectory f-name)
>                 article
>                 ;; TODO: Provide a cleaner way of producing final
>                 ;; article numbers for the various backends.
>                 (if (string-match-p "\\`[[:digit:]]+\\'" article)
> 		    (string-to-number article)
> 		  (nnmaildir-base-name-to-article-number
> 		   (substring article 0 (string-match ":" article))
> 		   group (string-remove-prefix "nnmaildir:" server))))
>           (when (and (numberp article)
>                      (or (null groups)
>                          (member group groups)))
> 	    (push (list f-name article group score)
>                   artlist)))))
>     ;; Are we running an additional grep query?
>     (when-let ((grep-reg (alist-get 'grep query)))
>       (setq artlist (gnus-search-grep-search engine artlist grep-reg)))
>     ;; Munge into the list of vectors expected by nnselect.
>     (mapcar (pcase-lambda (`(,_ ,article ,group ,score))
>               (vector group article
>                       (if (numberp score)
> 			  score
> 		        (string-to-number score))))
>             artlist)))
>
>
>

-- 
7LL-1 Make peace with your past so it doesn’t spoil your present.


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

* Re: Help testing gnus-search fixups
  2021-06-28 23:15   ` Eric Abrahamsen
  2021-06-29  6:04     ` Thomas Alexander Gerds
@ 2021-06-29  9:30     ` Adam Sjøgren
  2021-06-29  9:55       ` Adam Sjøgren
  1 sibling, 1 reply; 15+ messages in thread
From: Adam Sjøgren @ 2021-06-29  9:30 UTC (permalink / raw)
  To: ding

Eric writes:

> Adam Sjøgren <asjo@koldfront.dk> writes:
>
>> Eric writes:
>>
>>> if you're using notmuch or namazu, please help me give this a whirl!
>>
>> Which version of Emacs is your patch against? I just tried applying it
>> to master@2a33fc8d19 and got:
>
> It applies to up-to-date master

Ok, I did a git pull just before trying - well, no ~25 minutes before
trying the patch, as I built in between. D'oh!

I will give it another go, thanks!


  Best regards,

    Adam

-- 
 "Here comes my darling, saying hello you                   Adam Sjøgren
  Why you look so worried, whats a-wrong with you?"    asjo@koldfront.dk



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

* Re: Help testing gnus-search fixups
  2021-06-29  9:30     ` Adam Sjøgren
@ 2021-06-29  9:55       ` Adam Sjøgren
  2021-06-29 11:03         ` Thomas Alexander Gerds
  0 siblings, 1 reply; 15+ messages in thread
From: Adam Sjøgren @ 2021-06-29  9:55 UTC (permalink / raw)
  To: ding

Adam writes:

> I will give it another go, thanks!

It applied cleanly to master@6b4043833c. I am using notmuch with nnml,
by having (gnus-search-engine gnus-search-notmuch) on my nnml-method.

If I put point at nnml:normal in the *Group* buffer and press GG and
type "Honda" as the search term, with the patch, I get:

  Doing notmuch query on (nnml:normal)...
  Group nnselect:nnselect-87lf6tq9s1.fsf contains no messages

Without the patch, I get a group with search results:

   O. [ 859: Honda Danmark          ] [...]
   O. [1028: Honda Danmark          ]
   O  [ 819: Honda Danmark          ]
   O  [1048: Honda Danmark          ]
   O  [1002: Honda Danmark          ]
   O  [1001: Honda Danmark          ]
   O  [1006: Honda Danmark          ]
   O  [ 855: Honda Danmark          ]
   O  [1113: Honda Danmark          ]

Doing the search with notmuch on the commandline:

  $ notmuch search Honda | cut -d' ' -f1-5
  thread:000000000000f081   2020-06-09 [1/1]
  thread:000000000000ed7b   2020-06-03 [1/1]
  thread:000000000000ee56   2020-05-26 [1/1]
  thread:000000000000efe8   2020-05-18 [1/1]
  thread:000000000000ed4e   2020-03-12 [1/1]
  thread:000000000000ed4d   2020-03-12 [1/1]
  thread:000000000000ef24   2020-01-14 [1/1]
  thread:00000000000123a0   2019-09-05 [1/1]
  thread:000000000001239f   2019-09-04 [1/1]
  thread:000000000000c7cf   2018-10-22 [1/1]
  thread:0000000000014de8   2018-03-04 [1/1]
  thread:0000000000004e0d   2014-05-16 [1/1]
  thread:0000000000009b6a   2000-04-12 [1/1(2)]

(I cut the output to remove information I don't want to share).


  Best regards,

    Adam

-- 
 "the office is quiet now, i am at the desk,                Adam Sjøgren
  preparing to landscape for another day."             asjo@koldfront.dk



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

* Re: Help testing gnus-search fixups
  2021-06-29  9:55       ` Adam Sjøgren
@ 2021-06-29 11:03         ` Thomas Alexander Gerds
  2021-06-29 11:13           ` Adam Sjøgren
  0 siblings, 1 reply; 15+ messages in thread
From: Thomas Alexander Gerds @ 2021-06-29 11:03 UTC (permalink / raw)
  To: Adam Sjøgren; +Cc: ding

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


the attached patch applied to the patched file fixes this for me. 


Adam Sjøgren <asjo@koldfront.dk> writes:

> Adam writes:
>
>> I will give it another go, thanks!
>
> It applied cleanly to master@6b4043833c. I am using notmuch with nnml,
> by having (gnus-search-engine gnus-search-notmuch) on my nnml-method.
>
> If I put point at nnml:normal in the *Group* buffer and press GG and
> type "Honda" as the search term, with the patch, I get:
>
>   Doing notmuch query on (nnml:normal)...
>   Group nnselect:nnselect-87lf6tq9s1.fsf contains no messages
>
> Without the patch, I get a group with search results:
>
>    O. [ 859: Honda Danmark          ] [...]
>    O. [1028: Honda Danmark          ]
>    O  [ 819: Honda Danmark          ]
>    O  [1048: Honda Danmark          ]
>    O  [1002: Honda Danmark          ]
>    O  [1001: Honda Danmark          ]
>    O  [1006: Honda Danmark          ]
>    O  [ 855: Honda Danmark          ]
>    O  [1113: Honda Danmark          ]
>
> Doing the search with notmuch on the commandline:
>
>   $ notmuch search Honda | cut -d' ' -f1-5
>   thread:000000000000f081   2020-06-09 [1/1]
>   thread:000000000000ed7b   2020-06-03 [1/1]
>   thread:000000000000ee56   2020-05-26 [1/1]
>   thread:000000000000efe8   2020-05-18 [1/1]
>   thread:000000000000ed4e   2020-03-12 [1/1]
>   thread:000000000000ed4d   2020-03-12 [1/1]
>   thread:000000000000ef24   2020-01-14 [1/1]
>   thread:00000000000123a0   2019-09-05 [1/1]
>   thread:000000000001239f   2019-09-04 [1/1]
>   thread:000000000000c7cf   2018-10-22 [1/1]
>   thread:0000000000014de8   2018-03-04 [1/1]
>   thread:0000000000004e0d   2014-05-16 [1/1]
>   thread:0000000000009b6a   2000-04-12 [1/1(2)]
>
> (I cut the output to remove information I don't want to share).
>
>
>   Best regards,
>
>     Adam

-- 
7LL-7 Smile, you don’t own all the problems in the world.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-Rework-gnus-search-indexed-parse-output.patch --]
[-- Type: text/x-diff; name="0002-Rework-gnus-search-indexed-parse-output.patch", Size: 403 bytes --]

--- gnus-search-old.el	2021-06-29 13:00:20.478849987 +0200
+++ gnus-search.el	2021-06-29 13:00:28.226895659 +0200
@@ -1373,6 +1373,7 @@
                    nil t)
 	          nil t)
 	         nil t))
+	  (setq group (gnus-group-full-name group server))
           (setq article (file-name-nondirectory f-name)
                 article
                 ;; TODO: Provide a cleaner way of producing final

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

* Re: Help testing gnus-search fixups
  2021-06-29 11:03         ` Thomas Alexander Gerds
@ 2021-06-29 11:13           ` Adam Sjøgren
  2021-06-29 11:25             ` Adam Sjøgren
  0 siblings, 1 reply; 15+ messages in thread
From: Adam Sjøgren @ 2021-06-29 11:13 UTC (permalink / raw)
  To: Thomas Alexander Gerds; +Cc: ding

Thomas writes:

> the attached patch applied to the patched file fixes this for me. 

That makes search work for me as well.


  👍,

   Adam

-- 
 "This is not going exactly as planned."                    Adam Sjøgren
                                                       asjo@koldfront.dk


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

* Re: Help testing gnus-search fixups
  2021-06-29 11:13           ` Adam Sjøgren
@ 2021-06-29 11:25             ` Adam Sjøgren
  2021-06-30 16:58               ` Eric Abrahamsen
  0 siblings, 1 reply; 15+ messages in thread
From: Adam Sjøgren @ 2021-06-29 11:25 UTC (permalink / raw)
  To: ding

Adam writes:

> Thomas writes:
>
>> the attached patch applied to the patched file fixes this for me. 
>
> That makes search work for me as well.

In case Thomas' email doesn't reach the mailing list, here is the
combined diff I ended up with:

diff --git a/lisp/gnus/gnus-search.el b/lisp/gnus/gnus-search.el
index 70bde264c1..1b7b38a1cc 100644
--- a/lisp/gnus/gnus-search.el
+++ b/lisp/gnus/gnus-search.el
@@ -1351,68 +1351,59 @@ gnus-search-run-search
 
 (cl-defmethod gnus-search-indexed-parse-output ((engine gnus-search-indexed)
 						server query &optional groups)
-  (let ((prefix (slot-value engine 'remove-prefix))
-	(group-regexp (when groups
-			(mapconcat
-			 (lambda (group-name)
-			   (mapconcat #'regexp-quote
-				      (split-string
-				       (gnus-group-real-name group-name)
-				       "[.\\/]")
-				      "[.\\\\/]"))
-			 groups
-			 "\\|")))
-	artlist vectors article group)
+  (let ((prefix (or (slot-value engine 'remove-prefix)
+                    ""))
+	artlist article group)
     (goto-char (point-min))
+    ;; Prep prefix, we want to at least be removing the root
+    ;; filesystem separator.
+    (when (stringp prefix)
+      (setq prefix (file-name-as-directory
+                    (expand-file-name prefix "/"))))
     (while (not (or (eobp)
                     (looking-at-p
                      "\\(?:[[:space:]\n]+\\)?Process .+ finished")))
       (pcase-let ((`(,f-name ,score) (gnus-search-indexed-extract engine)))
 	(when (and f-name
                    (file-readable-p f-name)
-		   (null (file-directory-p f-name))
-		   (or (null groups)
-		       (and (gnus-search-single-p query)
-			    (alist-get 'thread query))
-		       (string-match-p group-regexp f-name)))
-	  (push (list f-name score) artlist))))
+		   (null (file-directory-p f-name)))
+          (setq group
+                (replace-regexp-in-string
+	         "[/\\]" "."
+	         (replace-regexp-in-string
+	          "/?\\(cur\\|new\\|tmp\\)?/\\'" ""
+	          (replace-regexp-in-string
+	           "\\`\\." ""
+	           (string-remove-prefix
+                    prefix (file-name-directory f-name))
+                   nil t)
+	          nil t)
+	         nil t))
+	  (setq group (gnus-group-full-name group server))
+          (setq article (file-name-nondirectory f-name)
+                article
+                ;; TODO: Provide a cleaner way of producing final
+                ;; article numbers for the various backends.
+                (if (string-match-p "\\`[[:digit:]]+\\'" article)
+		    (string-to-number article)
+		  (nnmaildir-base-name-to-article-number
+		   (substring article 0 (string-match ":" article))
+		   group (string-remove-prefix "nnmaildir:" server))))
+          (when (and (numberp article)
+                     (or (null groups)
+                         (member group groups)))
+	    (push (list f-name article group score)
+                  artlist)))))
     ;; Are we running an additional grep query?
     (when-let ((grep-reg (alist-get 'grep query)))
       (setq artlist (gnus-search-grep-search engine artlist grep-reg)))
-    ;; Prep prefix.
-    (when (and prefix (null (string-empty-p prefix)))
-      (setq prefix (file-name-as-directory (expand-file-name prefix))))
-    ;; Turn (file-name score) into [group article score].
-    (pcase-dolist (`(,f-name ,score) artlist)
-      (setq article (file-name-nondirectory f-name)
-	    group (file-name-directory f-name))
-      ;; Remove prefix.
-      (when prefix
-	(setq group (string-remove-prefix prefix group)))
-      ;; Break the directory name down until it's something that
-      ;; (probably) can be used as a group name.
-      (setq group
-	    (replace-regexp-in-string
-	     "[/\\]" "."
-	     (replace-regexp-in-string
-	      "/?\\(cur\\|new\\|tmp\\)?/\\'" ""
-	      (replace-regexp-in-string
-	       "^[./\\]" ""
-	       group nil t)
-	      nil t)
-	     nil t))
-
-      (push (vector (gnus-group-full-name group server)
-		    (if (string-match-p "\\`[[:digit:]]+\\'" article)
-			(string-to-number article)
-		      (nnmaildir-base-name-to-article-number
-		       (substring article 0 (string-match ":" article))
-		       group (string-remove-prefix "nnmaildir:" server)))
-		    (if (numberp score)
-			score
-		      (string-to-number score)))
-	    vectors))
-    vectors))
+    ;; Munge into the list of vectors expected by nnselect.
+    (mapcar (pcase-lambda (`(,_ ,article ,group ,score))
+              (vector group article
+                      (if (numberp score)
+			  score
+		        (string-to-number score))))
+            artlist)))
 
 (cl-defmethod gnus-search-indexed-extract ((_engine gnus-search-indexed))
   "Base implementation treats the whole line as a filename, and

-- 
 "Wiggle, wiggle, wiggle like a bowl of soup,               Adam Sjøgren
  Wiggle, wiggle, wiggle like a rolling hoop,          asjo@koldfront.dk



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

* Re: Help testing gnus-search fixups
  2021-06-28 16:42 Help testing gnus-search fixups Eric Abrahamsen
  2021-06-28 18:00 ` Adam Sjøgren
@ 2021-06-29 11:27 ` Eric S Fraga
  1 sibling, 0 replies; 15+ messages in thread
From: Eric S Fraga @ 2021-06-29 11:27 UTC (permalink / raw)
  To: ding

Hi Eric,
I'm very busy today and tomorrow but I'll try to give this a whirl soon.
-- 
Eric S Fraga via Emacs 28.0.50 & org 9.4.6 on Debian bullseye/sid



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

* Re: Help testing gnus-search fixups
  2021-06-29 11:25             ` Adam Sjøgren
@ 2021-06-30 16:58               ` Eric Abrahamsen
       [not found]                 ` <87bl6tuqcb.fsf@posteo.org>
  0 siblings, 1 reply; 15+ messages in thread
From: Eric Abrahamsen @ 2021-06-30 16:58 UTC (permalink / raw)
  To: Adam Sjøgren; +Cc: ding

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

Adam Sjøgren <asjo@koldfront.dk> writes:

> Adam writes:
>
>> Thomas writes:
>>
>>> the attached patch applied to the patched file fixes this for me. 
>>
>> That makes search work for me as well.
>
> In case Thomas' email doesn't reach the mailing list, here is the
> combined diff I ended up with:

Thanks to you both! That happened to slip by the test cases I was using,
and that's why we get extra testers! I've attached a new version of the
patch with your change applied; I'll give it a few days in case anyone
else wants to test it, then push.

Thanks,
Eric



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Rework-gnus-search-indexed-parse-output.patch --]
[-- Type: text/x-patch, Size: 5068 bytes --]

From b15fd217335f6711560ef88205ccfc262b4d2f25 Mon Sep 17 00:00:00 2001
From: Eric Abrahamsen <eric@ericabrahamsen.net>
Date: Sat, 26 Jun 2021 10:16:19 -0700
Subject: [PATCH] Rework gnus-search-indexed-parse-output

* lisp/gnus/gnus-search.el (gnus-search-indexed-parse-output): Be more
careful about matching filesystem paths to Gnus group names; make
absolutely sure that we only return valid article numbers.
---
 lisp/gnus/gnus-search.el | 95 ++++++++++++++++++----------------------
 1 file changed, 43 insertions(+), 52 deletions(-)

diff --git a/lisp/gnus/gnus-search.el b/lisp/gnus/gnus-search.el
index 70bde264c1..898b57bcef 100644
--- a/lisp/gnus/gnus-search.el
+++ b/lisp/gnus/gnus-search.el
@@ -1351,68 +1351,59 @@ gnus-search-run-search
 
 (cl-defmethod gnus-search-indexed-parse-output ((engine gnus-search-indexed)
 						server query &optional groups)
-  (let ((prefix (slot-value engine 'remove-prefix))
-	(group-regexp (when groups
-			(mapconcat
-			 (lambda (group-name)
-			   (mapconcat #'regexp-quote
-				      (split-string
-				       (gnus-group-real-name group-name)
-				       "[.\\/]")
-				      "[.\\\\/]"))
-			 groups
-			 "\\|")))
-	artlist vectors article group)
+  (let ((prefix (or (slot-value engine 'remove-prefix)
+                    ""))
+	artlist article group)
     (goto-char (point-min))
+    ;; Prep prefix, we want to at least be removing the root
+    ;; filesystem separator.
+    (when (stringp prefix)
+      (setq prefix (file-name-as-directory
+                    (expand-file-name prefix "/"))))
     (while (not (or (eobp)
                     (looking-at-p
                      "\\(?:[[:space:]\n]+\\)?Process .+ finished")))
       (pcase-let ((`(,f-name ,score) (gnus-search-indexed-extract engine)))
 	(when (and f-name
                    (file-readable-p f-name)
-		   (null (file-directory-p f-name))
-		   (or (null groups)
-		       (and (gnus-search-single-p query)
-			    (alist-get 'thread query))
-		       (string-match-p group-regexp f-name)))
-	  (push (list f-name score) artlist))))
+		   (null (file-directory-p f-name)))
+          (setq group
+                (replace-regexp-in-string
+	         "[/\\]" "."
+	         (replace-regexp-in-string
+	          "/?\\(cur\\|new\\|tmp\\)?/\\'" ""
+	          (replace-regexp-in-string
+	           "\\`\\." ""
+	           (string-remove-prefix
+                    prefix (file-name-directory f-name))
+                   nil t)
+	          nil t)
+	         nil t))
+          (setq group (gnus-group-full-name group server))
+          (setq article (file-name-nondirectory f-name)
+                article
+                ;; TODO: Provide a cleaner way of producing final
+                ;; article numbers for the various backends.
+                (if (string-match-p "\\`[[:digit:]]+\\'" article)
+		    (string-to-number article)
+		  (nnmaildir-base-name-to-article-number
+		   (substring article 0 (string-match ":" article))
+		   group (string-remove-prefix "nnmaildir:" server))))
+          (when (and (numberp article)
+                     (or (null groups)
+                         (member group groups)))
+	    (push (list f-name article group score)
+                  artlist)))))
     ;; Are we running an additional grep query?
     (when-let ((grep-reg (alist-get 'grep query)))
       (setq artlist (gnus-search-grep-search engine artlist grep-reg)))
-    ;; Prep prefix.
-    (when (and prefix (null (string-empty-p prefix)))
-      (setq prefix (file-name-as-directory (expand-file-name prefix))))
-    ;; Turn (file-name score) into [group article score].
-    (pcase-dolist (`(,f-name ,score) artlist)
-      (setq article (file-name-nondirectory f-name)
-	    group (file-name-directory f-name))
-      ;; Remove prefix.
-      (when prefix
-	(setq group (string-remove-prefix prefix group)))
-      ;; Break the directory name down until it's something that
-      ;; (probably) can be used as a group name.
-      (setq group
-	    (replace-regexp-in-string
-	     "[/\\]" "."
-	     (replace-regexp-in-string
-	      "/?\\(cur\\|new\\|tmp\\)?/\\'" ""
-	      (replace-regexp-in-string
-	       "^[./\\]" ""
-	       group nil t)
-	      nil t)
-	     nil t))
-
-      (push (vector (gnus-group-full-name group server)
-		    (if (string-match-p "\\`[[:digit:]]+\\'" article)
-			(string-to-number article)
-		      (nnmaildir-base-name-to-article-number
-		       (substring article 0 (string-match ":" article))
-		       group (string-remove-prefix "nnmaildir:" server)))
-		    (if (numberp score)
-			score
-		      (string-to-number score)))
-	    vectors))
-    vectors))
+    ;; Munge into the list of vectors expected by nnselect.
+    (mapcar (pcase-lambda (`(,_ ,article ,group ,score))
+              (vector group article
+                      (if (numberp score)
+			  score
+		        (string-to-number score))))
+            artlist)))
 
 (cl-defmethod gnus-search-indexed-extract ((_engine gnus-search-indexed))
   "Base implementation treats the whole line as a filename, and
-- 
2.32.0


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

* Re: Help testing gnus-search fixups
       [not found]                 ` <87bl6tuqcb.fsf@posteo.org>
@ 2021-07-23 15:18                   ` Eric Abrahamsen
       [not found]                     ` <87bl6sual8.fsf@posteo.org>
  0 siblings, 1 reply; 15+ messages in thread
From: Eric Abrahamsen @ 2021-07-23 15:18 UTC (permalink / raw)
  To: Alexandr Vityazev; +Cc: ding


On 07/23/21 13:12 PM, Alexandr Vityazev wrote:
> Hi!
>
> GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.24,
> cairo version 1.16.0). Gnus v5.13.
>
> I tested the new version of gnus-search-indexed-parse-output yesterday
> and the search with notmuch completely fails. gnus-verbose is set to
> 10. press GG and type any search query, with the patch, I get:
>
>   Doing /home/akagi/.guix-profile/bin/notmuch query on
>   (nnmaildir+posteo:Drafts nnmaildir+posteo:Inbox nnmaildir+posteo:Sent
>   nnmaildir+posteo:Trash nnmaildir+posteo:Notes)...
>   Group nnselect:nnselect-87lf5xus2s.fsf contains no messages

Hi, thanks for the report. Would you mind running your query on the
command line (using the same syntax as gnus-search does, mostly
output=files), and sending me the resulting list of file names
(off-list, if you prefer). I'll try to find out why the parsing process
is failing.


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

* Re: Help testing gnus-search fixups
       [not found]                     ` <87bl6sual8.fsf@posteo.org>
@ 2021-07-24 22:16                       ` Eric Abrahamsen
  2021-07-25 11:17                         ` Alexandr Vityazev
  0 siblings, 1 reply; 15+ messages in thread
From: Eric Abrahamsen @ 2021-07-24 22:16 UTC (permalink / raw)
  To: Alexandr Vityazev; +Cc: ding


On 07/23/21 18:52 PM, Alexandr Vityazev wrote:
> On 2021-07-23, 08:18 -0700, Eric Abrahamsen <eric@ericabrahamsen.net> wrote:
>
>> On 07/23/21 13:12 PM, Alexandr Vityazev wrote:
>>> Hi!
>>>
>>> GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.24,
>>> cairo version 1.16.0). Gnus v5.13.
>>>
>>> I tested the new version of gnus-search-indexed-parse-output yesterday
>>> and the search with notmuch completely fails. gnus-verbose is set to
>>> 10. press GG and type any search query, with the patch, I get:
>>>
>>>   Doing /home/akagi/.guix-profile/bin/notmuch query on
>>>   (nnmaildir+posteo:Drafts nnmaildir+posteo:Inbox nnmaildir+posteo:Sent
>>>   nnmaildir+posteo:Trash nnmaildir+posteo:Notes)...
>>>   Group nnselect:nnselect-87lf5xus2s.fsf contains no messages
>>
>> Hi, thanks for the report. Would you mind running your query on the
>> command line (using the same syntax as gnus-search does, mostly
>> output=files), and sending me the resulting list of file names
>> (off-list, if you prefer). I'll try to find out why the parsing process
>> is failing.
>>
>>
> notmuch search --output=files test
>
>
>
> in *Group* GG with "test" query, I get:
>
>    Doing /home/akagi/.guix-profile/bin/notmuch query on
>    (nnmaildir+posteo:Drafts nnmaildir+posteo:Inbox nnmaildir+posteo:Sent
>    nnmaildir+posteo:Trash nnmaildir+posteo:Notes nnmaildir+posteo:Junk)...
>    Group nnselect:nnselect-87czr8uasp.fsf contains no messages

Hmm, I'm not able to reproduce this: it's a little difficult since I
don't actually have the files on my machine, but as far as I can tell
the search results are being parsed correctly. Are you familiar with
edebug at all? Is there any chance I could get you to step through the
function and see where things are going off?

Thanks,
Eric


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

* Re: Help testing gnus-search fixups
  2021-07-24 22:16                       ` Eric Abrahamsen
@ 2021-07-25 11:17                         ` Alexandr Vityazev
  2021-07-25 22:11                           ` Eric Abrahamsen
  0 siblings, 1 reply; 15+ messages in thread
From: Alexandr Vityazev @ 2021-07-25 11:17 UTC (permalink / raw)
  To: Eric Abrahamsen; +Cc: ding

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

On 2021-07-24, 15:16 -0700, Eric Abrahamsen <eric@ericabrahamsen.net> wrote:

> On 07/23/21 18:52 PM, Alexandr Vityazev wrote:
>> On 2021-07-23, 08:18 -0700, Eric Abrahamsen <eric@ericabrahamsen.net> wrote:
>>
>>> On 07/23/21 13:12 PM, Alexandr Vityazev wrote:
>>>> Hi!
>>>>
>>>> GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.24,
>>>> cairo version 1.16.0). Gnus v5.13.
>>>>
>>>> I tested the new version of gnus-search-indexed-parse-output yesterday
>>>> and the search with notmuch completely fails. gnus-verbose is set to
>>>> 10. press GG and type any search query, with the patch, I get:
>>>>
>>>>   Doing /home/akagi/.guix-profile/bin/notmuch query on
>>>>   (nnmaildir+posteo:Drafts nnmaildir+posteo:Inbox nnmaildir+posteo:Sent
>>>>   nnmaildir+posteo:Trash nnmaildir+posteo:Notes)...
>>>>   Group nnselect:nnselect-87lf5xus2s.fsf contains no messages
>>>
>>> Hi, thanks for the report. Would you mind running your query on the
>>> command line (using the same syntax as gnus-search does, mostly
>>> output=files), and sending me the resulting list of file names
>>> (off-list, if you prefer). I'll try to find out why the parsing process
>>> is failing.
>>>
>>>
>> notmuch search --output=files test
>>
>>
>>
>> in *Group* GG with "test" query, I get:
>>
>>    Doing /home/akagi/.guix-profile/bin/notmuch query on
>>    (nnmaildir+posteo:Drafts nnmaildir+posteo:Inbox nnmaildir+posteo:Sent
>>    nnmaildir+posteo:Trash nnmaildir+posteo:Notes nnmaildir+posteo:Junk)...
>>    Group nnselect:nnselect-87czr8uasp.fsf contains no messages
>
> Hmm, I'm not able to reproduce this: it's a little difficult since I
> don't actually have the files on my machine, but as far as I can tell
> the search results are being parsed correctly. Are you familiar with
> edebug at all? Is there any chance I could get you to step through the
> function and see where things are going off?
>
> Thanks,
> Eric
>
>
I tested gnus-search-indexed-parse-output with edebug and found where
something is going on. The attached patch fixes the issue for me.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: gnus-search-indexed-parse-output --]
[-- Type: text/x-patch, Size: 1383 bytes --]

From bfba4f38289223c3e78b19a73ad9133d015f8420 Mon Sep 17 00:00:00 2001
From: Alexandr Vityazev <avityazev@posteo.org>
Date: Sun, 25 Jul 2021 14:09:39 +0300
Subject: [PATCH] gnus-search: gnus-search-indexed-parse-output: Fix search
 issue.

* lisp/gnus/gnus-search.el (gnus-search-indexed-parse-output):
Setting the group variable after the article variable.
---
 lisp/gnus/gnus-search.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/gnus/gnus-search.el b/lisp/gnus/gnus-search.el
index 39bde837b3..80c3ce62f1 100644
--- a/lisp/gnus/gnus-search.el
+++ b/lisp/gnus/gnus-search.el
@@ -1384,7 +1384,6 @@ Returns a list of [group article score] vectors."
                    nil t)
 	          nil t)
 	         nil t))
-          (setq group (gnus-group-full-name group server))
           (setq article (file-name-nondirectory f-name)
                 article
                 ;; TODO: Provide a cleaner way of producing final
@@ -1394,6 +1393,7 @@ Returns a list of [group article score] vectors."
 		  (nnmaildir-base-name-to-article-number
 		   (substring article 0 (string-match ":" article))
 		   group (string-remove-prefix "nnmaildir:" server))))
+          (setq group (gnus-group-full-name group server))
           (when (and (numberp article)
                      (or (null groups)
                          (member group groups)))
-- 
2.32.0


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


-- 
Best regards,
Alexandr Vityazev

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

* Re: Help testing gnus-search fixups
  2021-07-25 11:17                         ` Alexandr Vityazev
@ 2021-07-25 22:11                           ` Eric Abrahamsen
  0 siblings, 0 replies; 15+ messages in thread
From: Eric Abrahamsen @ 2021-07-25 22:11 UTC (permalink / raw)
  To: Alexandr Vityazev; +Cc: ding


On 07/25/21 11:17 AM, Alexandr Vityazev wrote:
> On 2021-07-24, 15:16 -0700, Eric Abrahamsen <eric@ericabrahamsen.net> wrote:
>
>> On 07/23/21 18:52 PM, Alexandr Vityazev wrote:
>>> On 2021-07-23, 08:18 -0700, Eric Abrahamsen <eric@ericabrahamsen.net> wrote:
>>>
>>>> On 07/23/21 13:12 PM, Alexandr Vityazev wrote:
>>>>> Hi!
>>>>>
>>>>> GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.24,
>>>>> cairo version 1.16.0). Gnus v5.13.
>>>>>
>>>>> I tested the new version of gnus-search-indexed-parse-output yesterday
>>>>> and the search with notmuch completely fails. gnus-verbose is set to
>>>>> 10. press GG and type any search query, with the patch, I get:
>>>>>
>>>>>   Doing /home/akagi/.guix-profile/bin/notmuch query on
>>>>>   (nnmaildir+posteo:Drafts nnmaildir+posteo:Inbox nnmaildir+posteo:Sent
>>>>>   nnmaildir+posteo:Trash nnmaildir+posteo:Notes)...
>>>>>   Group nnselect:nnselect-87lf5xus2s.fsf contains no messages
>>>>
>>>> Hi, thanks for the report. Would you mind running your query on the
>>>> command line (using the same syntax as gnus-search does, mostly
>>>> output=files), and sending me the resulting list of file names
>>>> (off-list, if you prefer). I'll try to find out why the parsing process
>>>> is failing.
>>>>
>>>>
>>> notmuch search --output=files test
>>>
>>>
>>>
>>> in *Group* GG with "test" query, I get:
>>>
>>>    Doing /home/akagi/.guix-profile/bin/notmuch query on
>>>    (nnmaildir+posteo:Drafts nnmaildir+posteo:Inbox nnmaildir+posteo:Sent
>>>    nnmaildir+posteo:Trash nnmaildir+posteo:Notes nnmaildir+posteo:Junk)...
>>>    Group nnselect:nnselect-87czr8uasp.fsf contains no messages
>>
>> Hmm, I'm not able to reproduce this: it's a little difficult since I
>> don't actually have the files on my machine, but as far as I can tell
>> the search results are being parsed correctly. Are you familiar with
>> edebug at all? Is there any chance I could get you to step through the
>> function and see where things are going off?
>>
>> Thanks,
>> Eric
>>
>>
> I tested gnus-search-indexed-parse-output with edebug and found where
> something is going on. The attached patch fixes the issue for me.

Wonderful, thank you very much for doing that. That code was added in to
fix a different bug, and I think we're just playing the usual "is the
group prefixed or not" game. I need to more thoroughly test the
primary/secondary select method cases, but at least we know what the
issue is -- thanks again for your debugging!

Eric


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

end of thread, other threads:[~2021-07-25 22:11 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-28 16:42 Help testing gnus-search fixups Eric Abrahamsen
2021-06-28 18:00 ` Adam Sjøgren
2021-06-28 23:15   ` Eric Abrahamsen
2021-06-29  6:04     ` Thomas Alexander Gerds
2021-06-29  9:30     ` Adam Sjøgren
2021-06-29  9:55       ` Adam Sjøgren
2021-06-29 11:03         ` Thomas Alexander Gerds
2021-06-29 11:13           ` Adam Sjøgren
2021-06-29 11:25             ` Adam Sjøgren
2021-06-30 16:58               ` Eric Abrahamsen
     [not found]                 ` <87bl6tuqcb.fsf@posteo.org>
2021-07-23 15:18                   ` Eric Abrahamsen
     [not found]                     ` <87bl6sual8.fsf@posteo.org>
2021-07-24 22:16                       ` Eric Abrahamsen
2021-07-25 11:17                         ` Alexandr Vityazev
2021-07-25 22:11                           ` Eric Abrahamsen
2021-06-29 11:27 ` Eric S Fraga

Gnus development mailing list

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://inbox.vuxu.org/ding

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 ding ding/ http://inbox.vuxu.org/ding \
		ding@inbox.vuxu.org
	public-inbox-index ding

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.vuxu.org/vuxu.archive.emacs.gnus.general


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git