* bug#44905: 27.1; Packages that customize xref-show-xrefs-function can break Dired's dired-do-find-regexp-and-replace [not found] <m1zh32imkw.fsf.ref@yahoo.es> @ 2020-11-27 20:14 ` Unknown 2020-11-28 19:04 ` Dmitry Gutov 0 siblings, 1 reply; 6+ messages in thread From: Unknown @ 2020-11-27 20:14 UTC (permalink / raw) To: 44905 Scenario: A package customizes xref-show-refs-function to use a different UI to show xref results. If that function does not return a valid xref buffer, then Dired's dired-do-find-regexp-and-replace (bound to Q by default) won't work anymore because it calls xref-query-replace-in-results on the buffer returned by xref--show-xrefs. A workaround is that the package explicitly calls xref--show-xref-buffer, which is done for example at https://github.com/alexmurray/ivy-xref/commit/aa97103ea8ce6ab8891e34deff7d43aa83fe36dd, but it doesn't feel like an ideal solution because of the duplicated work. Is there a way dired-do-find-regexp-and-replace could not depend on an actual xref buffer to perform the replacement? ^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#44905: 27.1; Packages that customize xref-show-xrefs-function can break Dired's dired-do-find-regexp-and-replace 2020-11-27 20:14 ` bug#44905: 27.1; Packages that customize xref-show-xrefs-function can break Dired's dired-do-find-regexp-and-replace Unknown @ 2020-11-28 19:04 ` Dmitry Gutov 2020-11-29 12:57 ` Unknown 0 siblings, 1 reply; 6+ messages in thread From: Dmitry Gutov @ 2020-11-28 19:04 UTC (permalink / raw) To: Daniel Martín, 44905 On 27.11.2020 22:14, Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors wrote: > > Scenario: A package customizes xref-show-refs-function to use a > different UI to show xref results. If that function does not return a > valid xref buffer, then Dired's dired-do-find-regexp-and-replace (bound > to Q by default) won't work anymore because it calls > xref-query-replace-in-results on the buffer returned by > xref--show-xrefs. Oh, indeed. Thanks for the report. > A workaround is that the package explicitly calls > xref--show-xref-buffer, which is done for example at > https://github.com/alexmurray/ivy-xref/commit/aa97103ea8ce6ab8891e34deff7d43aa83fe36dd, > but it doesn't feel like an ideal solution because of the duplicated > work. It's too bad this wasn't reported a year ago. > Is there a way dired-do-find-regexp-and-replace could not depend on an > actual xref buffer to perform the replacement? Well, there are options like: - Mandate that any xref-show-refs-function alternative has a corresponding feature that performs replacements in the matches. - Reimplement dired-do-find-regexp-and-replace entirely in terms of a different UI (e.g. create a faster dired-do-query-replace-regexp). But in this particular scenario we can just override xref-show-xrefs-function to use the default behavior, see below. While this change in the Right Thing(tm), I have to question the wisdom of setting xref-show-xrefs-function to an Ivy or Helm-based function, though. Those UIs serve to help you choose one item, whereas commands like dired-do-find-regexp and project-find-regexp show the user a list of matches, to interact with (usually) several of them. diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el index 94a2bbf1f3..4caafc8df6 100644 --- a/lisp/dired-aux.el +++ b/lisp/dired-aux.el @@ -3140,7 +3140,10 @@ dired-do-find-regexp-and-replace (query-replace-read-args "Query replace regexp in marked files" t t))) (list (nth 0 common) (nth 1 common)))) - (with-current-buffer (dired-do-find-regexp from) + (defvar xref-show-xrefs-function) + (with-current-buffer + (let ((xref-show-xrefs-function 'xref--show-xref-buffer)) + (dired-do-find-regexp from)) (xref-query-replace-in-results from to))) (defun dired-nondirectory-p (file) ^ permalink raw reply related [flat|nested] 6+ messages in thread
* bug#44905: 27.1; Packages that customize xref-show-xrefs-function can break Dired's dired-do-find-regexp-and-replace 2020-11-28 19:04 ` Dmitry Gutov @ 2020-11-29 12:57 ` Unknown 2020-11-30 1:00 ` Dmitry Gutov 0 siblings, 1 reply; 6+ messages in thread From: Unknown @ 2020-11-29 12:57 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 44905 Dmitry Gutov <dgutov@yandex.ru> writes: > > While this change in the Right Thing(tm), I have to question the > wisdom of setting xref-show-xrefs-function to an Ivy or Helm-based > function, though. Those UIs serve to help you choose one item, whereas > commands like dired-do-find-regexp and project-find-regexp show the > user a list of matches, to interact with (usually) several of them. I agree with you, Ivy/Helm may not be the best UX for Xref, but this particular problem can still happen even if the user returns a buffer that represents the Xref data in a different way. I think that xref-query-replace-in-results assumes certain invariants from the original *xref* buffer that are not documented. > > diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el > index 94a2bbf1f3..4caafc8df6 100644 > --- a/lisp/dired-aux.el > +++ b/lisp/dired-aux.el > @@ -3140,7 +3140,10 @@ dired-do-find-regexp-and-replace > (query-replace-read-args > "Query replace regexp in marked files" t t))) > (list (nth 0 common) (nth 1 common)))) > - (with-current-buffer (dired-do-find-regexp from) > + (defvar xref-show-xrefs-function) > + (with-current-buffer > + (let ((xref-show-xrefs-function 'xref--show-xref-buffer)) > + (dired-do-find-regexp from)) > (xref-query-replace-in-results from to))) > > (defun dired-nondirectory-p (file) LGTM, thanks. ^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#44905: 27.1; Packages that customize xref-show-xrefs-function can break Dired's dired-do-find-regexp-and-replace 2020-11-29 12:57 ` Unknown @ 2020-11-30 1:00 ` Dmitry Gutov 2020-11-30 15:26 ` Eli Zaretskii 0 siblings, 1 reply; 6+ messages in thread From: Dmitry Gutov @ 2020-11-30 1:00 UTC (permalink / raw) To: Daniel Martín; +Cc: 44905 On 29.11.2020 14:57, Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors wrote: > Dmitry Gutov <dgutov@yandex.ru> writes: >> >> While this change in the Right Thing(tm), I have to question the >> wisdom of setting xref-show-xrefs-function to an Ivy or Helm-based >> function, though. Those UIs serve to help you choose one item, whereas >> commands like dired-do-find-regexp and project-find-regexp show the >> user a list of matches, to interact with (usually) several of them. > > I agree with you, Ivy/Helm may not be the best UX for Xref, but this > particular problem can still happen even if the user returns a buffer > that represents the Xref data in a different way. I think that > xref-query-replace-in-results assumes certain invariants from the > original *xref* buffer that are not documented. True. >> >> diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el >> index 94a2bbf1f3..4caafc8df6 100644 >> --- a/lisp/dired-aux.el >> +++ b/lisp/dired-aux.el >> @@ -3140,7 +3140,10 @@ dired-do-find-regexp-and-replace >> (query-replace-read-args >> "Query replace regexp in marked files" t t))) >> (list (nth 0 common) (nth 1 common)))) >> - (with-current-buffer (dired-do-find-regexp from) >> + (defvar xref-show-xrefs-function) >> + (with-current-buffer >> + (let ((xref-show-xrefs-function 'xref--show-xref-buffer)) >> + (dired-do-find-regexp from)) >> (xref-query-replace-in-results from to))) >> >> (defun dired-nondirectory-p (file) > > LGTM, thanks. Eli, is this OK for Emacs 27.2? Here's also a slightly more future-proofed version that avoids referencing the function we might want to rename/change later: diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el index 94a2bbf1f3..26155190d4 100644 --- a/lisp/dired-aux.el +++ b/lisp/dired-aux.el @@ -3140,7 +3140,13 @@ dired-do-find-regexp-and-replace (query-replace-read-args "Query replace regexp in marked files" t t))) (list (nth 0 common) (nth 1 common)))) - (with-current-buffer (dired-do-find-regexp from) + (require 'xref) + (defvar xref-show-xrefs-function) + (with-current-buffer + (let ((xref-show-xrefs-function + ;; Some future-proofing (bug#44905). + (eval (car (get 'xref-show-xrefs-function 'standard-value))))) + (dired-do-find-regexp from)) (xref-query-replace-in-results from to))) (defun dired-nondirectory-p (file) ^ permalink raw reply related [flat|nested] 6+ messages in thread
* bug#44905: 27.1; Packages that customize xref-show-xrefs-function can break Dired's dired-do-find-regexp-and-replace 2020-11-30 1:00 ` Dmitry Gutov @ 2020-11-30 15:26 ` Eli Zaretskii 2020-12-01 1:50 ` Dmitry Gutov 0 siblings, 1 reply; 6+ messages in thread From: Eli Zaretskii @ 2020-11-30 15:26 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 44905, mardani29 > Cc: 44905@debbugs.gnu.org, Eli Zaretskii <eliz@gnu.org> > From: Dmitry Gutov <dgutov@yandex.ru> > Date: Mon, 30 Nov 2020 03:00:17 +0200 > > >> diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el > >> index 94a2bbf1f3..4caafc8df6 100644 > >> --- a/lisp/dired-aux.el > >> +++ b/lisp/dired-aux.el > >> @@ -3140,7 +3140,10 @@ dired-do-find-regexp-and-replace > >> (query-replace-read-args > >> "Query replace regexp in marked files" t t))) > >> (list (nth 0 common) (nth 1 common)))) > >> - (with-current-buffer (dired-do-find-regexp from) > >> + (defvar xref-show-xrefs-function) > >> + (with-current-buffer > >> + (let ((xref-show-xrefs-function 'xref--show-xref-buffer)) > >> + (dired-do-find-regexp from)) > >> (xref-query-replace-in-results from to))) > >> > >> (defun dired-nondirectory-p (file) > > > > LGTM, thanks. > > Eli, is this OK for Emacs 27.2? Yes, thanks. > Here's also a slightly more future-proofed version that avoids > referencing the function we might want to rename/change later: That, too. ^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#44905: 27.1; Packages that customize xref-show-xrefs-function can break Dired's dired-do-find-regexp-and-replace 2020-11-30 15:26 ` Eli Zaretskii @ 2020-12-01 1:50 ` Dmitry Gutov 0 siblings, 0 replies; 6+ messages in thread From: Dmitry Gutov @ 2020-12-01 1:50 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 44905-done, mardani29 Version: 27.2 On 30.11.2020 17:26, Eli Zaretskii wrote: >> Cc: 44905@debbugs.gnu.org, Eli Zaretskii <eliz@gnu.org> >> From: Dmitry Gutov <dgutov@yandex.ru> >> Date: Mon, 30 Nov 2020 03:00:17 +0200 >> >>>> diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el >>>> index 94a2bbf1f3..4caafc8df6 100644 >>>> --- a/lisp/dired-aux.el >>>> +++ b/lisp/dired-aux.el >>>> @@ -3140,7 +3140,10 @@ dired-do-find-regexp-and-replace >>>> (query-replace-read-args >>>> "Query replace regexp in marked files" t t))) >>>> (list (nth 0 common) (nth 1 common)))) >>>> - (with-current-buffer (dired-do-find-regexp from) >>>> + (defvar xref-show-xrefs-function) >>>> + (with-current-buffer >>>> + (let ((xref-show-xrefs-function 'xref--show-xref-buffer)) >>>> + (dired-do-find-regexp from)) >>>> (xref-query-replace-in-results from to))) >>>> >>>> (defun dired-nondirectory-p (file) >>> >>> LGTM, thanks. >> >> Eli, is this OK for Emacs 27.2? > > Yes, thanks. > >> Here's also a slightly more future-proofed version that avoids >> referencing the function we might want to rename/change later: > > That, too. Thanks. Pushed to the release branch (will get merged to master in due time), commit 749e4b7e0b. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-12-01 1:50 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <m1zh32imkw.fsf.ref@yahoo.es>
2020-11-27 20:14 ` bug#44905: 27.1; Packages that customize xref-show-xrefs-function can break Dired's dired-do-find-regexp-and-replace Unknown
2020-11-28 19:04 ` Dmitry Gutov
2020-11-29 12:57 ` Unknown
2020-11-30 1:00 ` Dmitry Gutov
2020-11-30 15:26 ` Eli Zaretskii
2020-12-01 1:50 ` Dmitry Gutov
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).