Package: emacs;
Reported by: Björn Bidar <bjorn.bidar <at> thaodan.de>
Date: Thu, 2 Jan 2025 23:37:02 UTC
Severity: wishlist
Found in version 31.0.50
View this message in rfc822 format
From: Eli Zaretskii <eliz <at> gnu.org> To: acohen <at> ust.hk, Björn Bidar <bjorn.bidar <at> thaodan.de> Cc: eric <at> ericabrahamsen.net, 75305 <at> debbugs.gnu.org Subject: bug#75305: 31.0.50; gnus-refer-thread-use-search isn't exact enough about how the current group is searched Date: Sun, 09 Mar 2025 11:25:16 +0200
Ping! Can we please make progress with this bug?
> From: Björn Bidar <bjorn.bidar <at> thaodan.de>
> Cc: Eli Zaretskii <eliz <at> gnu.org>, <eric <at> ericabrahamsen.net>,
> <75305 <at> debbugs.gnu.org>
> Date: Mon, 24 Feb 2025 18:07:48 +0200
>
> Andrew Cohen <acohen <at> ust.hk> writes:
>
> >>>>>> "BB" == Björn Bidar <bjorn.bidar <at> thaodan.de> writes:
> >
> > BB> Andrew Cohen <acohen <at> ust.hk> writes:
> > >>>>>>> "EZ" == Eli Zaretskii <eliz <at> gnu.org> writes:
> > >>
> > EZ> Ping! Can we please make some progress here?
> > >>
> > >> All set to commit, I think. I have been running with the
> > >> following in my personal tree for the past few weeks without
> > >> issue. If no objections we can commit.
> >
> > BB> I would have hoped there would be a response to my mail. Further
> > BB> I don't mind to refactor the contribution to the issues you
> > BB> mentioned but I don't think it is great to just take over IMHO.
> >
> >
> > I sincerely apologize. It was never my intent to take over and I
> > definitely don't want to make you feel disenfranchised. I am perfectly
> > happy to let you continue to manage this issue.
>
> No problem for me it only about the communication of this. I.e. someone
> contributes and an existing developer comes with a counter patch that
> incorporates the work from themselves.
>
> > Eli contacted me about this since I wrote and maintain the code that is
> > implicated in this change. In the end I thought that the tiny change
> > (along with the defcustom and documentation changes that you suggested)
> > in gnus-sum.el was the best way to introduce the feature that you
> > suggested.
>
> Well your patch effectively does the same minus including 'current as a
> possible value for gnus-refer-thread-use-search without any other values
> and slightly different documentation.
> As long as the bug is fixed I am fine, there are other bugs there to
> fix/patches to review for Gnus.
>
> Acknowledgment/credit helps. It also helpful to label the reviewers
> and/or those who suggested a change or reviewed a patch with commit
> message trailer see: git-interpret-trailers(1) (the man page in context
> is for the helper tool which isn't a requirement to use these but it
> does explain the concept).
>
> > And again I apologize for not responding to your previous email.
> >
> >>In think this would be potentially to slow. That's what's happening in
> >>nnselect refer article which can cause that it searches on the whole
> >>server e.g. an imap server which means it could take hours to finish
> >>searching.
> >
> > This seems like something might be broken? I routinely search imap
> > servers with 100 groups and it finishes in a few seconds. Does your imap
> > server maintain a searchable index? All of them should, and then the
> > searching is basically instantaneous, aside from the cost of the network
> > round trip and the changing of groups.
>
> You mean like as in full-text search? I don't think I have that enabled.
> Good pointer.
>
> >>From my point of view when talking about a non-virtual group it does
> >>makes sense to refer to the "current" as in the group that the user is
> >>in when pressing A T.
> >>However I get the point of that not being idea for nnselect groups. To
> >>be honest I did not think of that when writing the patch. I agree using
> >>artgroup is better in this context. This also aligns with the original
> >>behavior I wanted to align with when writing the patch i.e. to include
> >>the groups listed in gnus-refer-thread-use-search but also include the
> >>articles which would be found if gnus-refer-thread-use-search was nil.
> >
> > Although I originally suggested this, I think I was mistaken for doing
> > so. The nnselect search routine does indeed collect the component groups
> > and search them. So this aligns the behavior of 'current with
> > gnus-refer-thread-use-search set to nil.
>
> Oh that matches in retrospect what I experienced to when trying to limit
> searches in nnselect-ephemeral groups to the groups the articles come
> from.
>
> >>From my personal point of the just that was the goal so I could e.g. put
> >>the send group into the list of groups searched when building the thread
> >>including the current group.
> >
> > Agreed.
> >
> > FWIW here is the patch with Eli's suggested fixes incorporated. Feel
> > free to discard---I won't be offended.
> >
> > From 13d4062453bd8de5718d1781a701f26e17248061 Mon Sep 17 00:00:00 2001
> > From: Andrew G Cohen <cohen <at> andy.bu.edu>
> > Date: Sun, 23 Feb 2025 11:18:37 +0800
> > Subject: [PATCH] Allow 'current in gnus-refer-thread-use-search
> >
> > The variable `gnus-refer-thread-use-search' controls which groups are
> > searched when finding articles in a thread. The symbol `current' can be
> > used to include the current group in those searched.
> >
> > * doc/misc/gnus.texi: Document the use of `current' in
> > `gnus-refer-thread-use-search'.
> > * lisp/gnus/gnus-sum.el (gnus-summary-refer-thread): Resolve the symbol
> > `current' to the group from which `gnus-summary-refer-thread' was
> > invoked.
> > ---
> > doc/misc/gnus.texi | 25 ++++++++++++++-------
> > lisp/gnus/gnus-sum.el | 51 +++++++++++++++++++++++++------------------
> > 2 files changed, 47 insertions(+), 29 deletions(-)
> >
> > diff --git a/doc/misc/gnus.texi b/doc/misc/gnus.texi
> > index bdfab7a8718..b09c059cf9a 100644
> > --- a/doc/misc/gnus.texi
> > +++ b/doc/misc/gnus.texi
> > @@ -10546,8 +10546,7 @@ Finding the Parent
> > @findex gnus-summary-refer-thread
> > @kindex A T @r{(Summary)}
> > Display the full thread where the current article appears
> > -(@code{gnus-summary-refer-thread}). By default this command looks for
> > -articles only in the current group. If the group belongs to a backend
> > +(@code{gnus-summary-refer-thread}). If the group belongs to a backend
> > that has an associated search engine, articles are found by searching.
> > In other cases each header in the current group must be fetched and
> > examined, so it usually takes a while. If you do it often, you may
> > @@ -10557,12 +10556,22 @@ Finding the Parent
> > course, it'll make group entry somewhat slow.
> >
> > @vindex gnus-refer-thread-use-search
> > -If @code{gnus-refer-thread-use-search} is @code{nil} (the default)
> > -then thread-referral only looks for articles in the current group. If
> > -this variable is @code{t} the server to which the current group
> > -belongs is searched (provided that searching is available for the
> > -server's backend). If this variable is a list of servers, each server
> > -in the list is searched.
> > +If @code{gnus-refer-thread-use-search} is @code{nil} (the default), then
> > +thread-referral only looks for articles in the current group.
> > +
> > +If this variable is @code{t}, and the server to which the current group
> > +belongs has an associated search engine, then thread-referral searches
> > +this server.
>
> It might be helpful to either link to the relevant Info node or to
> `gnus-search-default-engines' to provide context.
>
> > +
> > +This variable may also be a list of servers and groups to search. Each
> > +element of the list is of the form @w{@code{(@var{server} @var{group1}
> > +@var{group2}@dots{})}}, where @var{server} is the server and
> > +@var{group1}, @var{group2}@dots{} are the groups belonging to this
> > +server that are to be searched. If the @var{group}s are absent the
> > +entire server is searched.
> > +
> > +This list may also contain the special symbol @code{current}, which will
> > +add the current group to the list of groups to be searched.
> >
> > @vindex gnus-refer-thread-limit
> > The @code{gnus-refer-thread-limit} variable says how many old (i.e.,
> > diff --git a/lisp/gnus/gnus-sum.el b/lisp/gnus/gnus-sum.el
> > index 6294825b400..672e4e1a71d 100644
> > --- a/lisp/gnus/gnus-sum.el
> > +++ b/lisp/gnus/gnus-sum.el
> > @@ -145,15 +145,21 @@ 'gnus-refer-thread-use-nnir
> > (defcustom gnus-refer-thread-use-search nil
> > "Specify where to find articles when referring threads.
> > A nil value restricts searches for thread-related articles to the
> > -current group; a value of t searches all groups on the server; a
> > -list of servers and groups (where each element is a list whose
> > -car is the server and whose cdr is a list of groups on this
> > -server or nil to search the entire server) searches these
> > -server/groups. This may usefully be set as a group parameter."
>
> You removed the suggestion here. I suggest to move that to the end of
> the docstring.
>
> > - :version "28.1"
> > +current group; a value of t searches all groups on the server to which
> > +the group belongs; a list of servers and groups (where each element is a
> > +list whose car is the server and whose cdr is a list of groups on this
> > +server, or nil to search the entire server) searches just these
> > +server/groups.
> > +
> > +The list of server/groups may contain the symbol `current' which
> > +includes the current group in the list of those searched."
>
> Is server/groups or server and groups better? Personally I think using
> and instead of / is easier to read.
>
> > + :version "31.1"
> > :group 'gnus-thread
> > - :type '(restricted-sexp :match-alternatives
> > - (listp 't 'nil)))
> > + :type '(choice (const :tag "Current group" nil)
> > + (const :tag "All groups" t)
> > + (repeat :tag "Server and groups"
> > + (choice (const :tag "Current Group" current)
> > + (repeat :tag "Server and groups" string)))))
> >
> > (defcustom gnus-refer-thread-limit-to-thread nil
> > "If non-nil referring a thread will limit the summary buffer to
> > @@ -9014,25 +9020,28 @@ gnus-delete-duplicate-headers
> >
> > (defun gnus-summary-refer-thread (&optional limit)
> > "Fetch all articles in the current thread.
> > -A non-numeric prefix arg will search the entire server; without a
> > -prefix arg only the current group is searched. If the variable
> > -`gnus-refer-thread-use-search' is t the prefix arg has the
> > -reverse meaning. If searching is not enabled for the current
> > -group, fetch LIMIT (the numerical prefix) old headers. If LIMIT
> > -is non-numeric or nil fetch the number specified by the
> > -`gnus-refer-thread-limit' variable."
> > +Find articles in the current group by default. With a non-numeric
> > +prefix arg, search all groups on the current server. If the variable
> > +`gnus-refer-thread-use-search' is t, invert the effect of the prefix
> > +arg; if it is a list, search only those groups in the list. If the
> > +current group does not have a search engine enabled, find articles by
> > +fetching LIMIT (the numerical prefix) old headers; if LIMIT is
> > +non-numeric or nil fetch the number specified by
> > +`gnus-refer-thread-limit'."
> > (interactive "P" gnus-summary-mode)
> > (let* ((group gnus-newsgroup-name)
> > + (server (gnus-method-to-server (gnus-find-method-for-group group)))
> > (header (gnus-summary-article-header))
> > (id (mail-header-id header))
> > (gnus-inhibit-demon t)
> > (gnus-summary-ignore-duplicates t)
> > (gnus-refer-thread-use-search
> > - (if (or (null limit) (numberp limit))
> > - gnus-refer-thread-use-search
> > - (if (booleanp gnus-refer-thread-use-search)
> > - (not gnus-refer-thread-use-search)
> > - gnus-refer-thread-use-search))))
> > + (if (listp gnus-refer-thread-use-search)
> > + (cl-subst (list server group) 'current
> > + gnus-refer-thread-use-search)
> > + (if (or (null limit) (numberp limit))
> > + gnus-refer-thread-use-search
> > + (not gnus-refer-thread-use-search)))))
> > (gnus-summary-insert-articles
> > (cond
> > ;; If there is a backend-specific method, use it.
> > @@ -9041,7 +9050,7 @@ gnus-summary-refer-thread
> > (gnus-request-thread header group))
> > ;; If a search engine is configured, use it.
> > ((ignore-errors
> > - (gnus-search-server-to-engine (gnus-group-server group)))
> > + (gnus-search-server-to-engine server))
> > (gnus-search-thread header))
> > ;; Otherwise just retrieve some headers.
> > (t
> > --
> > 2.47.2
>
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.