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
Message #67 received at 75305 <at> debbugs.gnu.org (full text, mbox):
From: Björn Bidar <bjorn.bidar <at> thaodan.de> To: Andrew Cohen <acohen <at> ust.hk> Cc: eric <at> ericabrahamsen.net, 75305 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org> Subject: Re: bug#75305: 31.0.50; gnus-refer-thread-use-search isn't exact enough about how the current group is searched 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.