unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#53989: 29.0.50; Gnus searches broken
@ 2022-02-14  3:37 Michael Heerdegen
  2022-02-14  4:23 ` Eric Abrahamsen
  0 siblings, 1 reply; 29+ messages in thread
From: Michael Heerdegen @ 2022-02-14  3:37 UTC (permalink / raw)
  To: 53989; +Cc: eric


Hello,

G g in Gnus only gives me search groups containing no messages.  I have
verified that matches are collected successfully, but they are not
processed correctly.

When I load the gnus-search.el version of the parent commit of

| e7f6bb38dd Rework gnus-search-indexed-parse-output
| Eric Abrahamsen <eric@ericabrahamsen.net> Sat Jun 26 10:16:19 2021 -0700

searching works again.  When I load the version of the commit, it's
broken.  So with my setup that commit seems to have broken something.

What could be the problem?

TIA,

Michael.


In GNU Emacs 29.0.50 (build 17, x86_64-pc-linux-gnu, GTK+ Version 3.24.24, cairo version 1.16.0)
 of 2022-02-14 built on drachen
Repository revision: f2dae7e79c37a94069245319166b9e4193f2ccf7
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12011000
System Description: Debian GNU/Linux 11 (bullseye)






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

* bug#53989: 29.0.50; Gnus searches broken
  2022-02-14  3:37 bug#53989: 29.0.50; Gnus searches broken Michael Heerdegen
@ 2022-02-14  4:23 ` Eric Abrahamsen
  2022-02-14  4:59   ` Michael Heerdegen
  0 siblings, 1 reply; 29+ messages in thread
From: Eric Abrahamsen @ 2022-02-14  4:23 UTC (permalink / raw)
  To: Michael Heerdegen; +Cc: 53989

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Hello,
>
> G g in Gnus only gives me search groups containing no messages.  I have
> verified that matches are collected successfully, but they are not
> processed correctly.
>
> When I load the gnus-search.el version of the parent commit of
>
> | e7f6bb38dd Rework gnus-search-indexed-parse-output
> | Eric Abrahamsen <eric@ericabrahamsen.net> Sat Jun 26 10:16:19 2021 -0700
>
> searching works again.  When I load the version of the commit, it's
> broken.  So with my setup that commit seems to have broken something.
>
> What could be the problem?

The problem is the search output isn't being parsed correctly! Again.
Can you tell me which Gnus backend this is on, what the search engine
is, what search engine config you've got, and the full filepath of a
typical search result that isn't showing up in searches?

TIA,
Eric





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

* bug#53989: 29.0.50; Gnus searches broken
  2022-02-14  4:23 ` Eric Abrahamsen
@ 2022-02-14  4:59   ` Michael Heerdegen
  2022-02-14  5:19     ` Michael Heerdegen
  2022-02-14  6:05     ` Eric Abrahamsen
  0 siblings, 2 replies; 29+ messages in thread
From: Michael Heerdegen @ 2022-02-14  4:59 UTC (permalink / raw)
  To: Eric Abrahamsen; +Cc: 53989

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> Can you tell me which Gnus backend this is on, what the search engine
> is, what search engine config you've got, and the full filepath of a
> typical search result that isn't showing up in searches?

I was using mairix to search my local mail archives (nnml):

(setf (alist-get 'nnml gnus-search-default-engines)
      'gnus-search-mairix)
(setq gnus-search-mairix-remove-prefix
      (expand-file-name "Mail/archive/" "~"))

Matches look like

/home/micha/Mail/archive//sent/9076

Anything else you need to know?

When edebugging, the path stuff seemed to work ok, I saw that the
article numbers like "9076" were processed individually.  However I
didn't understand why the group names in the code were like ".sent",
".work", etc, with a leading dot, instead of "sent", "work".  At that
point I gave up.  I had the impression that the articles were not
correctly mapped to their groups and all were just discarded.

Michael.





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

* bug#53989: 29.0.50; Gnus searches broken
  2022-02-14  4:59   ` Michael Heerdegen
@ 2022-02-14  5:19     ` Michael Heerdegen
  2022-02-14  6:06       ` Eric Abrahamsen
  2022-02-14 22:11       ` Eric Abrahamsen
  2022-02-14  6:05     ` Eric Abrahamsen
  1 sibling, 2 replies; 29+ messages in thread
From: Michael Heerdegen @ 2022-02-14  5:19 UTC (permalink / raw)
  To: Eric Abrahamsen; +Cc: 53989

Michael Heerdegen <michael_heerdegen@web.de> writes:

> When edebugging, the path stuff seemed to work ok, I saw that the
> article numbers like "9076" were processed individually.

I had edebugged gnus-search-indexed-parse-output.

When I replace the (member group groups) test there with just t, the
search group gets a positive article count in the group buffer, unlike
before.  But I still can't read the articles in that created search
group.

If you have problems to identify the issue, I can send you a recorded
Edebug session privately if that could help.

Michael.






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

* bug#53989: 29.0.50; Gnus searches broken
  2022-02-14  4:59   ` Michael Heerdegen
  2022-02-14  5:19     ` Michael Heerdegen
@ 2022-02-14  6:05     ` Eric Abrahamsen
  1 sibling, 0 replies; 29+ messages in thread
From: Eric Abrahamsen @ 2022-02-14  6:05 UTC (permalink / raw)
  To: Michael Heerdegen; +Cc: 53989

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> Can you tell me which Gnus backend this is on, what the search engine
>> is, what search engine config you've got, and the full filepath of a
>> typical search result that isn't showing up in searches?
>
> I was using mairix to search my local mail archives (nnml):
>
> (setf (alist-get 'nnml gnus-search-default-engines)
>       'gnus-search-mairix)
> (setq gnus-search-mairix-remove-prefix
>       (expand-file-name "Mail/archive/" "~"))
>
> Matches look like
>
> /home/micha/Mail/archive//sent/9076

This double slash is odd?

> Anything else you need to know?
>
> When edebugging, the path stuff seemed to work ok, I saw that the
> article numbers like "9076" were processed individually.  However I
> didn't understand why the group names in the code were like ".sent",
> ".work", etc, with a leading dot, instead of "sent", "work".  At that
> point I gave up.  I had the impression that the articles were not
> correctly mapped to their groups and all were just discarded.

I'm not sure where the dot would come from, either, but that's a good
clue. I'll take a look.





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

* bug#53989: 29.0.50; Gnus searches broken
  2022-02-14  5:19     ` Michael Heerdegen
@ 2022-02-14  6:06       ` Eric Abrahamsen
  2022-02-14 22:11       ` Eric Abrahamsen
  1 sibling, 0 replies; 29+ messages in thread
From: Eric Abrahamsen @ 2022-02-14  6:06 UTC (permalink / raw)
  To: Michael Heerdegen; +Cc: 53989

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Michael Heerdegen <michael_heerdegen@web.de> writes:
>
>> When edebugging, the path stuff seemed to work ok, I saw that the
>> article numbers like "9076" were processed individually.
>
> I had edebugged gnus-search-indexed-parse-output.
>
> When I replace the (member group groups) test there with just t, the
> search group gets a positive article count in the group buffer, unlike
> before.  But I still can't read the articles in that created search
> group.

Right, the problem is the erroneous group names: even if you let the
search results pass through, the group name is still wrong, and the
article won't be found.





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

* bug#53989: 29.0.50; Gnus searches broken
  2022-02-14  5:19     ` Michael Heerdegen
  2022-02-14  6:06       ` Eric Abrahamsen
@ 2022-02-14 22:11       ` Eric Abrahamsen
  2022-02-14 22:41         ` Eric Abrahamsen
  2022-02-14 22:49         ` Michael Heerdegen
  1 sibling, 2 replies; 29+ messages in thread
From: Eric Abrahamsen @ 2022-02-14 22:11 UTC (permalink / raw)
  To: Michael Heerdegen; +Cc: 53989


On 02/14/22 06:19 AM, Michael Heerdegen wrote:
> Michael Heerdegen <michael_heerdegen@web.de> writes:
>
>> When edebugging, the path stuff seemed to work ok, I saw that the
>> article numbers like "9076" were processed individually.
>
> I had edebugged gnus-search-indexed-parse-output.
>
> When I replace the (member group groups) test there with just t, the
> search group gets a positive article count in the group buffer, unlike
> before.  But I still can't read the articles in that created search
> group.

Indeed, the problem is the doubled forward slash in your search results.
Does that actually come out of mairix? Any clue why that's in there?

We process the prefix to make sure that it ends in a forward slash
(which removes the first of the group name's two slashes), but the
second remains, and it is interpreted as a group name that looks like
this on the filesystem -- "computers/emacs" -- but should look like this
in Gnus -- "computers.emacs". Ie, internal slashes should be replaced
with a period.

We currently remove a leading ".", and I can additionally remove a
leading "/", but since in effect we've already done that, I'd like to
know where the doubled slash comes from.

This process is the bloody front line of Gnus' interface with a wild
world of search engines, and I'm happy to special-case the heck out of
it, but there is also something weird about the search results.

Thanks,
Eric





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

* bug#53989: 29.0.50; Gnus searches broken
  2022-02-14 22:11       ` Eric Abrahamsen
@ 2022-02-14 22:41         ` Eric Abrahamsen
  2022-02-14 23:06           ` Michael Heerdegen
  2022-02-14 22:49         ` Michael Heerdegen
  1 sibling, 1 reply; 29+ messages in thread
From: Eric Abrahamsen @ 2022-02-14 22:41 UTC (permalink / raw)
  To: Michael Heerdegen; +Cc: 53989


On 02/14/22 14:11 PM, Eric Abrahamsen wrote:
> On 02/14/22 06:19 AM, Michael Heerdegen wrote:
>> Michael Heerdegen <michael_heerdegen@web.de> writes:
>>
>>> When edebugging, the path stuff seemed to work ok, I saw that the
>>> article numbers like "9076" were processed individually.
>>
>> I had edebugged gnus-search-indexed-parse-output.
>>
>> When I replace the (member group groups) test there with just t, the
>> search group gets a positive article count in the group buffer, unlike
>> before.  But I still can't read the articles in that created search
>> group.
>
> Indeed, the problem is the doubled forward slash in your search results.
> Does that actually come out of mairix? Any clue why that's in there?
>
> We process the prefix to make sure that it ends in a forward slash
> (which removes the first of the group name's two slashes), but the
> second remains, and it is interpreted as a group name that looks like
> this on the filesystem -- "computers/emacs" -- but should look like this
> in Gnus -- "computers.emacs". Ie, internal slashes should be replaced
> with a period.
>
> We currently remove a leading ".", and I can additionally remove a
> leading "/", but since in effect we've already done that, I'd like to
> know where the doubled slash comes from.
>
> This process is the bloody front line of Gnus' interface with a wild
> world of search engines, and I'm happy to special-case the heck out of
> it, but there is also something weird about the search results.

Also, it looks like your bug#47130 is still open. Since we've moved on
to this one, maybe lets close that in the meantime?





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

* bug#53989: 29.0.50; Gnus searches broken
  2022-02-14 22:11       ` Eric Abrahamsen
  2022-02-14 22:41         ` Eric Abrahamsen
@ 2022-02-14 22:49         ` Michael Heerdegen
  2022-02-14 23:08           ` Eric Abrahamsen
  1 sibling, 1 reply; 29+ messages in thread
From: Michael Heerdegen @ 2022-02-14 22:49 UTC (permalink / raw)
  To: Eric Abrahamsen; +Cc: 53989

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> Does that actually come out of mairix? Any clue why that's in there?

I think it comes from mairix.  When I run

|  micha> mairix -r b:emacs

I get an output like

| /home/micha/Mail/archive//sent/8383
| /home/micha/Mail/archive//sent/8391
| ...

Without the -r arg ("raw output"), mairix fills a folder with symlinks.

That folder looks like

|  /home/micha/mairix:
|  drwxr-xr-x   2 micha micha 120K Feb 14 23:42 .
|  drwxr-xr-x 195 micha micha  60K Feb 14 23:42 ..
|  lrwxrwxrwx   1 micha micha   35 Feb 14 23:42 10002 -> /home/micha/Mail/archive//sent/2876
|  lrwxrwxrwx   1 micha micha   35 Feb 14 23:42 10005 -> /home/micha/Mail/archive//sent/2879

Visiting these symlinks succeeds, surprisingly, in Emacs, and in
Dolphin.  Is this double-slash syntax in symlink targets something
special or some kind of error that is just ignored?

This is my .mairixrc:

| base=~/Mail/archive
| mfolder=/home/micha/mairix
| mh=...
| mformat=mh
| omit=zz_mairix-*
| database=~/.mairixdatabase

Doesn't look like the slashes are from there.  And that's already all
setup I have, AFAIR. The folder names in /home/micha/Mail/archive just
look normally.

I tried to search the internet for the //-problem but didn't find
anything.

Michael.





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

* bug#53989: 29.0.50; Gnus searches broken
  2022-02-14 22:41         ` Eric Abrahamsen
@ 2022-02-14 23:06           ` Michael Heerdegen
  2022-02-14 23:22             ` Eric Abrahamsen
  0 siblings, 1 reply; 29+ messages in thread
From: Michael Heerdegen @ 2022-02-14 23:06 UTC (permalink / raw)
  To: Eric Abrahamsen; +Cc: 53989

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> Also, it looks like your bug#47130 is still open. Since we've moved on
> to this one, maybe lets close that in the meantime?

Did you already add the note to the documentation that we seemed to have
talked about in that report?  AFAIR, the note should say that the
binding of `gnus-search-mairix-remove-prefix' must correspond to the
path of the mail archive, not to some mairix folder.

Michael.





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

* bug#53989: 29.0.50; Gnus searches broken
  2022-02-14 22:49         ` Michael Heerdegen
@ 2022-02-14 23:08           ` Eric Abrahamsen
  2022-02-14 23:21             ` Michael Heerdegen
  0 siblings, 1 reply; 29+ messages in thread
From: Eric Abrahamsen @ 2022-02-14 23:08 UTC (permalink / raw)
  To: Michael Heerdegen; +Cc: 53989

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> Does that actually come out of mairix? Any clue why that's in there?
>
> I think it comes from mairix.  When I run
>
> |  micha> mairix -r b:emacs
>
> I get an output like
>
> | /home/micha/Mail/archive//sent/8383
> | /home/micha/Mail/archive//sent/8391
> | ...
>
> Without the -r arg ("raw output"), mairix fills a folder with symlinks.
>
> That folder looks like
>
> |  /home/micha/mairix:
> |  drwxr-xr-x   2 micha micha 120K Feb 14 23:42 .
> |  drwxr-xr-x 195 micha micha  60K Feb 14 23:42 ..
> |  lrwxrwxrwx   1 micha micha   35 Feb 14 23:42 10002 -> /home/micha/Mail/archive//sent/2876
> |  lrwxrwxrwx   1 micha micha   35 Feb 14 23:42 10005 -> /home/micha/Mail/archive//sent/2879
>
> Visiting these symlinks succeeds, surprisingly, in Emacs, and in
> Dolphin.  Is this double-slash syntax in symlink targets something
> special or some kind of error that is just ignored?

I guess it isn't symlink specific, since it's part of mairix's raw
output, too.

I was also surprised to see that Emacs handles a path like that just
fine. Looks like `directory-file-name' will remove as many trailing
slashes as you have on a filepath, and since that function gets used
somewhere inside most of the other filepath functions, it ends up
working.

Luckily `file-name-split' is one of those other functions, so instead of
doing gross regular expression munging I'll switch to using Science™ and
treat the path like an actual file name. Instead of adding cruft we can
fix your bug and make the whole thing more reliable at the same time.

(mapconcat #'identity
	   (remove "" (file-name-split f-name))
	   ".")

Maybe this will create more bugs, but let's see!

Hmm, maybe I'll float this on gnus.general first...





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

* bug#53989: 29.0.50; Gnus searches broken
  2022-02-14 23:08           ` Eric Abrahamsen
@ 2022-02-14 23:21             ` Michael Heerdegen
  2022-02-14 23:24               ` Eric Abrahamsen
  0 siblings, 1 reply; 29+ messages in thread
From: Michael Heerdegen @ 2022-02-14 23:21 UTC (permalink / raw)
  To: Eric Abrahamsen; +Cc: 53989

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> Hmm, maybe I'll float this on gnus.general first...

Would be good to know if this is a bug in mairix, or somewhere else.
Unfortunately Debian only offers one built of mairix.

I wonder if anyone else is seeing what I see.

Michael.





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

* bug#53989: 29.0.50; Gnus searches broken
  2022-02-14 23:06           ` Michael Heerdegen
@ 2022-02-14 23:22             ` Eric Abrahamsen
  0 siblings, 0 replies; 29+ messages in thread
From: Eric Abrahamsen @ 2022-02-14 23:22 UTC (permalink / raw)
  To: Michael Heerdegen; +Cc: 53989


On 02/15/22 00:06 AM, Michael Heerdegen wrote:
> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> Also, it looks like your bug#47130 is still open. Since we've moved on
>> to this one, maybe lets close that in the meantime?
>
> Did you already add the note to the documentation that we seemed to have
> talked about in that report?  AFAIR, the note should say that the
> binding of `gnus-search-mairix-remove-prefix' must correspond to the
> path of the mail archive, not to some mairix folder.

Done!





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

* bug#53989: 29.0.50; Gnus searches broken
  2022-02-14 23:21             ` Michael Heerdegen
@ 2022-02-14 23:24               ` Eric Abrahamsen
  2022-02-14 23:31                 ` Michael Heerdegen
  0 siblings, 1 reply; 29+ messages in thread
From: Eric Abrahamsen @ 2022-02-14 23:24 UTC (permalink / raw)
  To: Michael Heerdegen; +Cc: 53989


On 02/15/22 00:21 AM, Michael Heerdegen wrote:
> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> Hmm, maybe I'll float this on gnus.general first...
>
> Would be good to know if this is a bug in mairix, or somewhere else.
> Unfortunately Debian only offers one built of mairix.
>
> I wonder if anyone else is seeing what I see.

I'll ask at the same time I post the patch.





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

* bug#53989: 29.0.50; Gnus searches broken
  2022-02-14 23:24               ` Eric Abrahamsen
@ 2022-02-14 23:31                 ` Michael Heerdegen
  2022-02-14 23:35                   ` Eric Abrahamsen
  2022-02-14 23:40                   ` Michael Heerdegen
  0 siblings, 2 replies; 29+ messages in thread
From: Michael Heerdegen @ 2022-02-14 23:31 UTC (permalink / raw)
  To: Eric Abrahamsen; +Cc: 53989

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> I'll ask at the same time I post the patch.

Thanks.  Meanwhile, I changed

base=~/Mail/archive

to

base=~/Mail/archive/

in my .mairixrc and rebuilt the database.  Now the matches have three
slashes, like in "/home/micha/Mail/archive///sent/995".

Michael.





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

* bug#53989: 29.0.50; Gnus searches broken
  2022-02-14 23:31                 ` Michael Heerdegen
@ 2022-02-14 23:35                   ` Eric Abrahamsen
  2022-02-14 23:40                   ` Michael Heerdegen
  1 sibling, 0 replies; 29+ messages in thread
From: Eric Abrahamsen @ 2022-02-14 23:35 UTC (permalink / raw)
  To: 53989

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> I'll ask at the same time I post the patch.
>
> Thanks.  Meanwhile, I changed
>
> base=~/Mail/archive
>
> to
>
> base=~/Mail/archive/
>
> in my .mairixrc and rebuilt the database.  Now the matches have three
> slashes, like in "/home/micha/Mail/archive///sent/995".

Ha! But never fear, `file-name-split' removes as many slashes as you've got.






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

* bug#53989: 29.0.50; Gnus searches broken
  2022-02-14 23:31                 ` Michael Heerdegen
  2022-02-14 23:35                   ` Eric Abrahamsen
@ 2022-02-14 23:40                   ` Michael Heerdegen
  2022-02-14 23:45                     ` Eric Abrahamsen
  1 sibling, 1 reply; 29+ messages in thread
From: Michael Heerdegen @ 2022-02-14 23:40 UTC (permalink / raw)
  To: Eric Abrahamsen; +Cc: 53989

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
> > I'll ask at the same time I post the patch.
>
> Thanks.  Meanwhile, I changed
>
> base=~/Mail/archive
>
> to
>
> base=~/Mail/archive/
>
> in my .mairixrc and rebuilt the database.  Now the matches have three
> slashes, like in "/home/micha/Mail/archive///sent/995".

I now tried with

| mh=*

instead of

| mh=...

and that gives me the correct number of slashes, and searching in Gnus
finally works.

Seems mairix is just not very smart when expanding file names.

Michael.





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

* bug#53989: 29.0.50; Gnus searches broken
  2022-02-14 23:40                   ` Michael Heerdegen
@ 2022-02-14 23:45                     ` Eric Abrahamsen
  2022-02-15  0:37                       ` Andreas Schwab
  2022-02-15  2:48                       ` Michael Heerdegen
  0 siblings, 2 replies; 29+ messages in thread
From: Eric Abrahamsen @ 2022-02-14 23:45 UTC (permalink / raw)
  To: 53989

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Michael Heerdegen <michael_heerdegen@web.de> writes:
>
>> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>>
>> > I'll ask at the same time I post the patch.
>>
>> Thanks.  Meanwhile, I changed
>>
>> base=~/Mail/archive
>>
>> to
>>
>> base=~/Mail/archive/
>>
>> in my .mairixrc and rebuilt the database.  Now the matches have three
>> slashes, like in "/home/micha/Mail/archive///sent/995".
>
> I now tried with
>
> | mh=*
>
> instead of
>
> | mh=...
>
> and that gives me the correct number of slashes, and searching in Gnus
> finally works.
>
> Seems mairix is just not very smart when expanding file names.

Oh... hmm. I wonder if we should actually change the parsing then or
not? What do you think?






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

* bug#53989: 29.0.50; Gnus searches broken
  2022-02-14 23:45                     ` Eric Abrahamsen
@ 2022-02-15  0:37                       ` Andreas Schwab
  2022-02-15  1:29                         ` Eric Abrahamsen
  2022-02-15  2:48                       ` Michael Heerdegen
  1 sibling, 1 reply; 29+ messages in thread
From: Andreas Schwab @ 2022-02-15  0:37 UTC (permalink / raw)
  To: Eric Abrahamsen; +Cc: 53989

On Feb 14 2022, Eric Abrahamsen wrote:

> Oh... hmm. I wonder if we should actually change the parsing then or
> not? What do you think?

There is a similar problem in gud, see gud-find-file.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."





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

* bug#53989: 29.0.50; Gnus searches broken
  2022-02-15  0:37                       ` Andreas Schwab
@ 2022-02-15  1:29                         ` Eric Abrahamsen
  0 siblings, 0 replies; 29+ messages in thread
From: Eric Abrahamsen @ 2022-02-15  1:29 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: 53989


On 02/15/22 01:37 AM, Andreas Schwab wrote:
> On Feb 14 2022, Eric Abrahamsen wrote:
>
>> Oh... hmm. I wonder if we should actually change the parsing then or
>> not? What do you think?
>
> There is a similar problem in gud, see gud-find-file.

Huh, interesting. On the one hand, this particular error was (arguably)
caused by user config, or at least it was solvable though user config.
On the other hand, having your email searches fail when they wouldn't
have to is very annoying. I really hate this part of the code.





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

* bug#53989: 29.0.50; Gnus searches broken
  2022-02-14 23:45                     ` Eric Abrahamsen
  2022-02-15  0:37                       ` Andreas Schwab
@ 2022-02-15  2:48                       ` Michael Heerdegen
  2022-02-15  3:45                         ` Eric Abrahamsen
  1 sibling, 1 reply; 29+ messages in thread
From: Michael Heerdegen @ 2022-02-15  2:48 UTC (permalink / raw)
  To: Eric Abrahamsen; +Cc: 53989

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> Oh... hmm. I wonder if we should actually change the parsing then or
> not? What do you think?

`expand-file-name' collapses multiple consecutive slashes into a single
slash - that's documented.  Can we just map it over the names delivered
by mairix?

Michael.





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

* bug#53989: 29.0.50; Gnus searches broken
  2022-02-15  2:48                       ` Michael Heerdegen
@ 2022-02-15  3:45                         ` Eric Abrahamsen
  2022-02-15  4:20                           ` Michael Heerdegen
  0 siblings, 1 reply; 29+ messages in thread
From: Eric Abrahamsen @ 2022-02-15  3:45 UTC (permalink / raw)
  To: Michael Heerdegen; +Cc: 53989


On 02/15/22 03:48 AM, Michael Heerdegen wrote:
> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> Oh... hmm. I wonder if we should actually change the parsing then or
>> not? What do you think?
>
> `expand-file-name' collapses multiple consecutive slashes into a single
> slash - that's documented.  Can we just map it over the names delivered
> by mairix?

Oh, that's not bad. If you mess up your mairix config again and then
eval this:

(cl-defmethod gnus-search-indexed-extract :around
  ((_engine gnus-search-indexed))
  (let ((ret (cl-call-next-method)))
    (cl-callf expand-file-name (car ret) "/")
    ret))

Does everything work properly?





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

* bug#53989: 29.0.50; Gnus searches broken
  2022-02-15  3:45                         ` Eric Abrahamsen
@ 2022-02-15  4:20                           ` Michael Heerdegen
  2022-02-15 23:23                             ` Eric Abrahamsen
  0 siblings, 1 reply; 29+ messages in thread
From: Michael Heerdegen @ 2022-02-15  4:20 UTC (permalink / raw)
  To: Eric Abrahamsen; +Cc: 53989

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> Oh, that's not bad. If you mess up your mairix config again and then
> eval this:
>
> (cl-defmethod gnus-search-indexed-extract :around
>   ((_engine gnus-search-indexed))
>   (let ((ret (cl-call-next-method)))
>     (cl-callf expand-file-name (car ret) "/")
>     ret))
>
> Does everything work properly?

Yes, I think so, looks good.

Michael.





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

* bug#53989: 29.0.50; Gnus searches broken
  2022-02-15  4:20                           ` Michael Heerdegen
@ 2022-02-15 23:23                             ` Eric Abrahamsen
  2022-02-18  2:10                               ` Michael Heerdegen
  0 siblings, 1 reply; 29+ messages in thread
From: Eric Abrahamsen @ 2022-02-15 23:23 UTC (permalink / raw)
  To: Michael Heerdegen; +Cc: 53989

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


On 02/15/22 05:20 AM, Michael Heerdegen wrote:
> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> Oh, that's not bad. If you mess up your mairix config again and then
>> eval this:
>>
>> (cl-defmethod gnus-search-indexed-extract :around
>>   ((_engine gnus-search-indexed))
>>   (let ((ret (cl-call-next-method)))
>>     (cl-callf expand-file-name (car ret) "/")
>>     ret))
>>
>> Does everything work properly?
>
> Yes, I think so, looks good.

Okay, it turns out that also lets us drop most of the regexp silliness
from the parsing stuff. Would you mind trying the attached patch and
confirm that it doesn't break anything? I'll try to make this the last
time I mess with search results parsing...


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

diff --git a/lisp/gnus/gnus-search.el b/lisp/gnus/gnus-search.el
index 605d8a34e9..871163c38b 100644
--- a/lisp/gnus/gnus-search.el
+++ b/lisp/gnus/gnus-search.el
@@ -1396,17 +1396,22 @@ gnus-search-indexed-parse-output
                    (file-readable-p f-name)
 		   (null (file-directory-p f-name)))
           (setq group
-                (replace-regexp-in-string
-	         "[/\\]" "."
-	         (replace-regexp-in-string
-	          "/?\\(cur\\|new\\|tmp\\)?/\\'" ""
+                (delete
+                 ""
+                 (file-name-split
 	          (replace-regexp-in-string
-	           "\\`\\." ""
-	           (string-remove-prefix
+	           "\\`\\." "" ; Why do we do this?
+                   (string-remove-prefix
                     prefix (file-name-directory f-name))
-                   nil t)
-	          nil t)
-	         nil t))
+                   nil t)))
+                ;; Turn file name segments into Gnus group name.
+                group (mapconcat
+                       #'identity
+                       (if (member (car (last group))
+                                   '("new" "tmp" "cur"))
+                           (nbutlast group)
+                         group)
+                       "."))
           (setq article (file-name-nondirectory f-name)
                 article
                 ;; TODO: Provide a cleaner way of producing final
@@ -1434,6 +1439,14 @@ gnus-search-indexed-parse-output
 		 (string-to-number score))))
             artlist)))
 
+(cl-defmethod gnus-search-indexed-extract :around
+  ((_engine gnus-search-indexed))
+  (let ((ret (cl-call-next-method)))
+    ;; We run `expand-file-name' here in order to collapse multiple
+    ;; consecutive directory separators.
+    (cl-callf expand-file-name (car ret))
+    ret))
+
 (cl-defmethod gnus-search-indexed-extract ((_engine gnus-search-indexed))
   "Base implementation treats the whole line as a filename, and
 fudges a relevancy score of 100."

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

* bug#53989: 29.0.50; Gnus searches broken
  2022-02-15 23:23                             ` Eric Abrahamsen
@ 2022-02-18  2:10                               ` Michael Heerdegen
  2022-02-18 16:01                                 ` Eric Abrahamsen
  0 siblings, 1 reply; 29+ messages in thread
From: Michael Heerdegen @ 2022-02-18  2:10 UTC (permalink / raw)
  To: Eric Abrahamsen; +Cc: 53989

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> Okay, it turns out that also lets us drop most of the regexp silliness
> from the parsing stuff. Would you mind trying the attached patch and
> confirm that it doesn't break anything? I'll try to make this the last
> time I mess with search results parsing...

Works so far for me.

| +(cl-defmethod gnus-search-indexed-extract :around
| +  ((_engine gnus-search-indexed))
| +  (let ((ret (cl-call-next-method)))

Why do you need an :around method here - why don't you just add that to
the primary method?

| +    ;; We run `expand-file-name' here in order to collapse multiple
| +    ;; consecutive directory separators.
| +    (cl-callf expand-file-name (car ret))
| +    ret))

If I were you I would add to the comment that mairix may return such
multi-separator file names - else it's not clear why that collapsing is
necessary at that point.


Thanks,

Michael.





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

* bug#53989: 29.0.50; Gnus searches broken
  2022-02-18  2:10                               ` Michael Heerdegen
@ 2022-02-18 16:01                                 ` Eric Abrahamsen
  2022-02-19  1:07                                   ` Michael Heerdegen
  0 siblings, 1 reply; 29+ messages in thread
From: Eric Abrahamsen @ 2022-02-18 16:01 UTC (permalink / raw)
  To: Michael Heerdegen; +Cc: 53989


On 02/18/22 03:10 AM, Michael Heerdegen wrote:
> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> Okay, it turns out that also lets us drop most of the regexp silliness
>> from the parsing stuff. Would you mind trying the attached patch and
>> confirm that it doesn't break anything? I'll try to make this the last
>> time I mess with search results parsing...
>
> Works so far for me.
>
> | +(cl-defmethod gnus-search-indexed-extract :around
> | +  ((_engine gnus-search-indexed))
> | +  (let ((ret (cl-call-next-method)))
>
> Why do you need an :around method here - why don't you just add that to
> the primary method?

Hmm... I think I was originally trying to do more in this method --
prefix removal and all that. You're right this isn't necessary.

> | +    ;; We run `expand-file-name' here in order to collapse multiple
> | +    ;; consecutive directory separators.
> | +    (cl-callf expand-file-name (car ret))
> | +    ret))
>
> If I were you I would add to the comment that mairix may return such
> multi-separator file names - else it's not clear why that collapsing is
> necessary at that point.

I think I'd rather leave it like this. We don't know if the behavior
might appear under other circumstances, and in the end the real problem
is multiple separators, not mairix.





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

* bug#53989: 29.0.50; Gnus searches broken
  2022-02-18 16:01                                 ` Eric Abrahamsen
@ 2022-02-19  1:07                                   ` Michael Heerdegen
  2022-02-19  1:20                                     ` Eric Abrahamsen
  0 siblings, 1 reply; 29+ messages in thread
From: Michael Heerdegen @ 2022-02-19  1:07 UTC (permalink / raw)
  To: Eric Abrahamsen; +Cc: 53989

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> I think I'd rather leave it like this. We don't know if the behavior
> might appear under other circumstances, and in the end the real problem
> is multiple separators, not mairix.

Ok, fine.

And I see that the change has been installed.

So we are done here, right?


Thanks,

Michael.





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

* bug#53989: 29.0.50; Gnus searches broken
  2022-02-19  1:07                                   ` Michael Heerdegen
@ 2022-02-19  1:20                                     ` Eric Abrahamsen
  2022-02-19  1:29                                       ` Michael Heerdegen
  0 siblings, 1 reply; 29+ messages in thread
From: Eric Abrahamsen @ 2022-02-19  1:20 UTC (permalink / raw)
  To: Michael Heerdegen; +Cc: 53989


On 02/19/22 02:07 AM, Michael Heerdegen wrote:
> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> I think I'd rather leave it like this. We don't know if the behavior
>> might appear under other circumstances, and in the end the real problem
>> is multiple separators, not mairix.
>
> Ok, fine.
>
> And I see that the change has been installed.
>
> So we are done here, right?

We are! I hereby certify this area of the code bug-free, guaranteed for
ten years.





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

* bug#53989: 29.0.50; Gnus searches broken
  2022-02-19  1:20                                     ` Eric Abrahamsen
@ 2022-02-19  1:29                                       ` Michael Heerdegen
  0 siblings, 0 replies; 29+ messages in thread
From: Michael Heerdegen @ 2022-02-19  1:29 UTC (permalink / raw)
  To: Eric Abrahamsen; +Cc: 53989-close

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> > So we are done here, right?
>
> We are! I hereby certify this area of the code bug-free, guaranteed for
> ten years.

Very cool!  I hope I don't have to make use of the guarantee, but it's
good to have it.

So - thanks for fixing!

Michael.





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

end of thread, other threads:[~2022-02-19  1:29 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-14  3:37 bug#53989: 29.0.50; Gnus searches broken Michael Heerdegen
2022-02-14  4:23 ` Eric Abrahamsen
2022-02-14  4:59   ` Michael Heerdegen
2022-02-14  5:19     ` Michael Heerdegen
2022-02-14  6:06       ` Eric Abrahamsen
2022-02-14 22:11       ` Eric Abrahamsen
2022-02-14 22:41         ` Eric Abrahamsen
2022-02-14 23:06           ` Michael Heerdegen
2022-02-14 23:22             ` Eric Abrahamsen
2022-02-14 22:49         ` Michael Heerdegen
2022-02-14 23:08           ` Eric Abrahamsen
2022-02-14 23:21             ` Michael Heerdegen
2022-02-14 23:24               ` Eric Abrahamsen
2022-02-14 23:31                 ` Michael Heerdegen
2022-02-14 23:35                   ` Eric Abrahamsen
2022-02-14 23:40                   ` Michael Heerdegen
2022-02-14 23:45                     ` Eric Abrahamsen
2022-02-15  0:37                       ` Andreas Schwab
2022-02-15  1:29                         ` Eric Abrahamsen
2022-02-15  2:48                       ` Michael Heerdegen
2022-02-15  3:45                         ` Eric Abrahamsen
2022-02-15  4:20                           ` Michael Heerdegen
2022-02-15 23:23                             ` Eric Abrahamsen
2022-02-18  2:10                               ` Michael Heerdegen
2022-02-18 16:01                                 ` Eric Abrahamsen
2022-02-19  1:07                                   ` Michael Heerdegen
2022-02-19  1:20                                     ` Eric Abrahamsen
2022-02-19  1:29                                       ` Michael Heerdegen
2022-02-14  6:05     ` Eric Abrahamsen

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

	https://git.savannah.gnu.org/cgit/emacs.git

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