unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [Patch] Add project.el command to replace symbol at point throughout project
@ 2022-01-11  7:45 Jon Eskin
  2022-01-11  7:51 ` Manuel Uberti
                   ` (2 more replies)
  0 siblings, 3 replies; 42+ messages in thread
From: Jon Eskin @ 2022-01-11  7:45 UTC (permalink / raw)
  To: emacs-devel


[-- Attachment #1.1: Type: text/plain, Size: 1404 bytes --]

Attached is a small patch adding command 'project-query-replace-at-point'
to project.el. The command is designed to improve the ergonomics of making
a project wide text replacement of a symbol at point.

Currently, if you want to make a project wide replacement of a symbol using
project.el, the best options I've found are:

- Mark the symbol you wish to replace
- Save symbol to kill ring with 'kill-ring-save'
- Enter command 'project-query-replace-regexp'
- Paste in the symbol, taking care to quote regex if necessary, and hit
return
- Enter the replacement string and hit return

or

- Mark the symbol you wish to replace
- Save symbol to kill ring with 'kill-ring-save'
- Place cursor on symbol and enter command 'project-find-regexp'
- Hit return at the next prompt to accept the default prompt
- Enter command 'xref-query-replace-in-results'
- Enter the replacement string and hit return

'project-query-replace-at-point' regex-quotes the symbol at point and then
calls into the fileloop-initialize-replace function used by the existing
project-query-replace-regexp command.

Replacing a symbol with 'project-query-replace-at-point' occurs as follows:

- Place cursor on symbol and enter command 'project-query-replace-at-point'
- Enter the replacement string and hit return

Let me know what you guys think. I haven't contributed before so please let
me know if I'm doing anything incorrectly.

[-- Attachment #1.2: Type: text/html, Size: 1596 bytes --]

[-- Attachment #2: 0001-Add-function-for-project-wide-replacement-of-symbol-.patch --]
[-- Type: text/x-patch, Size: 2934 bytes --]

From 9f962641c4e813e41d188f4000ab3f83ce3aba8e Mon Sep 17 00:00:00 2001
From: Jon Eskin <eskinjp@gmail.com>
Date: Mon, 10 Jan 2022 15:04:35 -0500
Subject: [PATCH] Add function for project-wide replacement of symbol at point.

* lisp/progmodes/project.el
(project-query-replace-at-point): New command.
(project--query-replace-at-point-read-args): New function.

* doc/emacs/maintaining.texi
(Project File Commands): Document 'project-query-replace-at-point'
---
 doc/emacs/maintaining.texi |  2 ++
 lisp/progmodes/project.el  | 24 ++++++++++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/doc/emacs/maintaining.texi b/doc/emacs/maintaining.texi
index 9a23f23e0e..65ab5cdca5 100644
--- a/doc/emacs/maintaining.texi
+++ b/doc/emacs/maintaining.texi
@@ -1690,6 +1690,8 @@ Project File Commands
 @item M-x project-search
 Interactively search for regexp matches in all files that belong to
 the current project.
+@item M-x project-query-replace-at-point
+Perform query-replace for the symbol at point.
 @item C-x p r
 Perform query-replace for a regexp in all files that belong to the
 current project (@code{project-query-replace-regexp}).
diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index eda19c46a3..acc6013066 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -849,6 +849,16 @@ project--read-regexp
     (read-regexp "Find regexp" (and sym (regexp-quote sym))
                  project-regexp-history-variable)))
 
+(defun project--query-replace-at-point-read-args ()
+  "Prompt for a string name to replace the symbol at point in the project."
+  (let* ((sym (thing-at-point 'symbol t))
+         (regex-quoted-sym (and sym (regexp-quote sym)))
+         (replace (query-replace-read-to
+                   regex-quoted-sym
+                   "Throughout project replace symbol:"
+                   nil)))
+    (list regex-quoted-sym replace)))
+
 ;;;###autoload
 (defun project-find-file (&optional include-all)
   "Visit a file (with completion) in the current project.
@@ -1065,6 +1075,20 @@ project-search
    regexp (project-files (project-current t)) 'default)
   (fileloop-continue))
 
+;;;###autoload
+(defun project-query-replace-at-point (thing-at-point replacement)
+  "Query-replace THING-AT-POINT in all files of the project.
+Stops when a match is found and prompts for whether to replace it.
+If you exit the `query-replace', you can later continue the
+`query-replace' loop using the command \\[fileloop-continue]."
+  (interactive
+   (pcase-let ((`(,thing-at-point ,replacement)
+                (project--query-replace-at-point-read-args)))
+     (list thing-at-point replacement)))
+  (fileloop-initialize-replace
+   thing-at-point replacement (project-files (project-current t)) 'default)
+  (fileloop-continue))
+
 ;;;###autoload
 (defun project-query-replace-regexp (from to)
   "Query-replace REGEXP in all the files of the project.
-- 
2.34.1


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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-01-11  7:45 [Patch] Add project.el command to replace symbol at point throughout project Jon Eskin
@ 2022-01-11  7:51 ` Manuel Uberti
  2022-01-11  9:50   ` Jon Eskin
  2022-01-11 13:10 ` Eli Zaretskii
  2022-01-12  3:42 ` Dmitry Gutov
  2 siblings, 1 reply; 42+ messages in thread
From: Manuel Uberti @ 2022-01-11  7:51 UTC (permalink / raw)
  To: Jon Eskin, emacs-devel

On 11/01/22 08:45, Jon Eskin wrote:
> Attached is a small patch adding command 'project-query-replace-at-point' to 
> project.el. The command is designed to improve the ergonomics of making a 
> project wide text replacement of a symbol at point.
> 
> Currently, if you want to make a project wide replacement of a symbol using 
> project.el, the best options I've found are:
> 
> - Mark the symbol you wish to replace
> - Save symbol to kill ring with 'kill-ring-save'
> - Enter command 'project-query-replace-regexp'
> - Paste in the symbol, taking care to quote regex if necessary, and hit return
> - Enter the replacement string and hit return
> 
> or
> 
> - Mark the symbol you wish to replace
> - Save symbol to kill ring with 'kill-ring-save'
> - Place cursor on symbol and enter command 'project-find-regexp'
> - Hit return at the next prompt to accept the default prompt
> - Enter command 'xref-query-replace-in-results'
> - Enter the replacement string and hit return
> 
> 'project-query-replace-at-point' regex-quotes the symbol at point and then calls 
> into the fileloop-initialize-replace function used by the existing 
> project-query-replace-regexp command.
> 
> Replacing a symbol with 'project-query-replace-at-point' occurs as follows:
> 
> - Place cursor on symbol and enter command 'project-query-replace-at-point'
> - Enter the replacement string and hit return
> 
> Let me know what you guys think. I haven't contributed before so please let me 
> know if I'm doing anything incorrectly.
> 

I usually use the `project-find-regexp` approach you described, but I agree a 
simpler solution would be nicer.

-- 
Manuel Uberti
www.manueluberti.eu



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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-01-11  7:51 ` Manuel Uberti
@ 2022-01-11  9:50   ` Jon Eskin
  2022-01-12  3:43     ` Dmitry Gutov
  0 siblings, 1 reply; 42+ messages in thread
From: Jon Eskin @ 2022-01-11  9:50 UTC (permalink / raw)
  To: Manuel Uberti; +Cc: emacs-devel

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

That's a great function, I love seeing all the results in an xref buffer. I
previously explored adding a command to the xref similar to
'xref-query-replace-in-results' that pre-populates the FROM field with the
item you were searching for when you executed 'project-find-regexp'.

Unfortunately I couldn't find a way to make that work- I couldn't find a
function or variable referring to the item being cross referenced in the
context of an xref buffer. If I understand correctly, xref only seems to be
concerned with logic to deal with xref results that it is provided. With
that assumption I couldn't really think of a way to architect a good
solution without modifying xref in a way that violates [what I understood
to be] its intent. I'm definitely open to suggestions on how to make that
work, though.

On Tue, Jan 11, 2022 at 2:51 AM Manuel Uberti <manuel.uberti@inventati.org>
wrote:

> On 11/01/22 08:45, Jon Eskin wrote:
> > Attached is a small patch adding command
> 'project-query-replace-at-point' to
> > project.el. The command is designed to improve the ergonomics of making
> a
> > project wide text replacement of a symbol at point.
> >
> > Currently, if you want to make a project wide replacement of a symbol
> using
> > project.el, the best options I've found are:
> >
> > - Mark the symbol you wish to replace
> > - Save symbol to kill ring with 'kill-ring-save'
> > - Enter command 'project-query-replace-regexp'
> > - Paste in the symbol, taking care to quote regex if necessary, and hit
> return
> > - Enter the replacement string and hit return
> >
> > or
> >
> > - Mark the symbol you wish to replace
> > - Save symbol to kill ring with 'kill-ring-save'
> > - Place cursor on symbol and enter command 'project-find-regexp'
> > - Hit return at the next prompt to accept the default prompt
> > - Enter command 'xref-query-replace-in-results'
> > - Enter the replacement string and hit return
> >
> > 'project-query-replace-at-point' regex-quotes the symbol at point and
> then calls
> > into the fileloop-initialize-replace function used by the existing
> > project-query-replace-regexp command.
> >
> > Replacing a symbol with 'project-query-replace-at-point' occurs as
> follows:
> >
> > - Place cursor on symbol and enter command
> 'project-query-replace-at-point'
> > - Enter the replacement string and hit return
> >
> > Let me know what you guys think. I haven't contributed before so please
> let me
> > know if I'm doing anything incorrectly.
> >
>
> I usually use the `project-find-regexp` approach you described, but I
> agree a
> simpler solution would be nicer.
>
> --
> Manuel Uberti
> www.manueluberti.eu
>


-- 
Jon Eskin
(301) 675-6663

[-- Attachment #2: Type: text/html, Size: 3508 bytes --]

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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-01-11  7:45 [Patch] Add project.el command to replace symbol at point throughout project Jon Eskin
  2022-01-11  7:51 ` Manuel Uberti
@ 2022-01-11 13:10 ` Eli Zaretskii
  2022-01-11 15:15   ` Daniel Martín
  2022-01-12  3:46   ` Dmitry Gutov
  2022-01-12  3:42 ` Dmitry Gutov
  2 siblings, 2 replies; 42+ messages in thread
From: Eli Zaretskii @ 2022-01-11 13:10 UTC (permalink / raw)
  To: Jon Eskin; +Cc: emacs-devel

> From: Jon Eskin <eskinjp@gmail.com>
> Date: Tue, 11 Jan 2022 02:45:57 -0500
> 
> Attached is a small patch adding command 'project-query-replace-at-point' to project.el. The command is
> designed to improve the ergonomics of making a project wide text replacement of a symbol at point.

Thanks.

I think we should have such a command outside project.el as well.
Would it be possible to base it on xref.el and related facilities, so
that, for example, one could rename a symbol in all the files
mentioned in TAGS table?



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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-01-11 13:10 ` Eli Zaretskii
@ 2022-01-11 15:15   ` Daniel Martín
  2022-01-11 17:17     ` Eli Zaretskii
  2022-01-12  3:46   ` Dmitry Gutov
  1 sibling, 1 reply; 42+ messages in thread
From: Daniel Martín @ 2022-01-11 15:15 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Jon Eskin, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Jon Eskin <eskinjp@gmail.com>
>> Date: Tue, 11 Jan 2022 02:45:57 -0500
>> 
>> Attached is a small patch adding command 'project-query-replace-at-point' to project.el. The command is
>> designed to improve the ergonomics of making a project wide text replacement of a symbol at point.
>
> Thanks.
>
> I think we should have such a command outside project.el as well.
> Would it be possible to base it on xref.el and related facilities, so
> that, for example, one could rename a symbol in all the files
> mentioned in TAGS table?

How would it be different from tags-query-replace?



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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-01-11 15:15   ` Daniel Martín
@ 2022-01-11 17:17     ` Eli Zaretskii
  0 siblings, 0 replies; 42+ messages in thread
From: Eli Zaretskii @ 2022-01-11 17:17 UTC (permalink / raw)
  To: Daniel Martín; +Cc: eskinjp, emacs-devel

> From: Daniel Martín <mardani29@yahoo.es>
> Cc: Jon Eskin <eskinjp@gmail.com>,  emacs-devel@gnu.org
> Date: Tue, 11 Jan 2022 16:15:46 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Jon Eskin <eskinjp@gmail.com>
> >> Date: Tue, 11 Jan 2022 02:45:57 -0500
> >> 
> >> Attached is a small patch adding command 'project-query-replace-at-point' to project.el. The command is
> >> designed to improve the ergonomics of making a project wide text replacement of a symbol at point.
> >
> > Thanks.
> >
> > I think we should have such a command outside project.el as well.
> > Would it be possible to base it on xref.el and related facilities, so
> > that, for example, one could rename a symbol in all the files
> > mentioned in TAGS table?
> 
> How would it be different from tags-query-replace?

Maybe it wouldn't.



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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-01-11  7:45 [Patch] Add project.el command to replace symbol at point throughout project Jon Eskin
  2022-01-11  7:51 ` Manuel Uberti
  2022-01-11 13:10 ` Eli Zaretskii
@ 2022-01-12  3:42 ` Dmitry Gutov
  2022-01-12  8:03   ` Jon Eskin
  2 siblings, 1 reply; 42+ messages in thread
From: Dmitry Gutov @ 2022-01-12  3:42 UTC (permalink / raw)
  To: Jon Eskin, emacs-devel

Hi!

On 11.01.2022 09:45, Jon Eskin wrote:
> Attached is a small patch adding command 
> 'project-query-replace-at-point' to project.el. The command is designed 
> to improve the ergonomics of making a project wide text replacement of a 
> symbol at point.
> 
> Currently, if you want to make a project wide replacement of a symbol 
> using project.el, the best options I've found are:

> or
> 
> - Mark the symbol you wish to replace
> - Save symbol to kill ring with 'kill-ring-save'

You don't need these two steps, do you? Just do the rest, and it should 
work.

> - Place cursor on symbol and enter command 'project-find-regexp'
> - Hit return at the next prompt to accept the default prompt
> - Enter command 'xref-query-replace-in-results'
> - Enter the replacement string and hit return

I agree it could use some more optimization still.

> 'project-query-replace-at-point' regex-quotes the symbol at point and 
> then calls into the fileloop-initialize-replace function used by the 
> existing project-query-replace-regexp command.
> 
> Replacing a symbol with 'project-query-replace-at-point' occurs as follows:
> 
> - Place cursor on symbol and enter command 'project-query-replace-at-point'
> - Enter the replacement string and hit return
> 
> Let me know what you guys think. I haven't contributed before so please 
> let me know if I'm doing anything incorrectly.

If we're trying to improve project-query-replace-regexp, why not make it 
use the symbol at point by default?

You would use the same command, but would be able to press RET to have 
the default regexp (symbol at point) used as FROM. I don't have a patch 
yet, but it might be something that all callers of 
query-replace-read-args might benefit from.

And you can actually do this right now:

- Enter command 'project-query-replace-regexp'
- Press M-n, having the symbol at point picked up as FROM, hit return
- Enter the replacement string and hit return



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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-01-11  9:50   ` Jon Eskin
@ 2022-01-12  3:43     ` Dmitry Gutov
  2022-01-12  9:42       ` Jon Eskin
  0 siblings, 1 reply; 42+ messages in thread
From: Dmitry Gutov @ 2022-01-12  3:43 UTC (permalink / raw)
  To: Jon Eskin, Manuel Uberti; +Cc: emacs-devel

On 11.01.2022 11:50, Jon Eskin wrote:
> That's a great function, I love seeing all the results in an xref 
> buffer. I previously explored adding a command to the xref similar to 
> 'xref-query-replace-in-results' that pre-populates the FROM field with 
> the item you were searching for when you executed 'project-find-regexp'.

Would it help to have an easier version of 
xref-query-replace-in-results, or do we really need a separate command 
called project-find-regexp-and-replace?



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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-01-11 13:10 ` Eli Zaretskii
  2022-01-11 15:15   ` Daniel Martín
@ 2022-01-12  3:46   ` Dmitry Gutov
  2022-01-12 12:45     ` Eli Zaretskii
  1 sibling, 1 reply; 42+ messages in thread
From: Dmitry Gutov @ 2022-01-12  3:46 UTC (permalink / raw)
  To: Eli Zaretskii, Jon Eskin; +Cc: emacs-devel

On 11.01.2022 15:10, Eli Zaretskii wrote:
> I think we should have such a command outside project.el as well.
> Would it be possible to base it on xref.el and related facilities, so
> that, for example, one could rename a symbol in all the files
> mentioned in TAGS table?

We could add a command like xref-find-references-and-replace.

Or you can do a search with M-? and then press 'r'. Have you tried that?



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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-01-12  3:42 ` Dmitry Gutov
@ 2022-01-12  8:03   ` Jon Eskin
  2022-01-12 19:43     ` Dmitry Gutov
  0 siblings, 1 reply; 42+ messages in thread
From: Jon Eskin @ 2022-01-12  8:03 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: emacs-devel

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

Hello! :)

> On Jan 11, 2022, at 10:42 PM, Dmitry Gutov <dgutov@yandex.ru> wrote:
> 
> Hi!
> 
> On 11.01.2022 09:45, Jon Eskin wrote:
>> Attached is a small patch adding command 'project-query-replace-at-point' to project.el. The command is designed to improve the ergonomics of making a project wide text replacement of a symbol at point.
>> Currently, if you want to make a project wide replacement of a symbol using project.el, the best options I've found are:
> 
>> or
>> - Mark the symbol you wish to replace
>> - Save symbol to kill ring with 'kill-ring-save'
> 
> You don't need these two steps, do you? Just do the rest, and it should work.
> 
>> - Place cursor on symbol and enter command 'project-find-regexp'
>> - Hit return at the next prompt to accept the default prompt
>> - Enter command 'xref-query-replace-in-results'
>> - Enter the replacement string and hit return
> 
> I agree it could use some more optimization still.
> 
>> 'project-query-replace-at-point' regex-quotes the symbol at point and then calls into the fileloop-initialize-replace function used by the existing project-query-replace-regexp command.
>> Replacing a symbol with 'project-query-replace-at-point' occurs as follows:
>> - Place cursor on symbol and enter command 'project-query-replace-at-point'
>> - Enter the replacement string and hit return
>> Let me know what you guys think. I haven't contributed before so please let me know if I'm doing anything incorrectly.

Sorry, yes, those two steps are not needed. My brain crossed a few wires.

> 
> If we're trying to improve project-query-replace-regexp, why not make it use the symbol at point by default?
> 

I thought about the option of using symbol at point by default, but I noticed that the command currently uses the history variable. I wondered if some people were relying on its current behavior in a way that I didn’t anticipate (especially since I’m pretty new to all this). If so, it would be an annoying change to have to worry about whether the cursor is on a symbol when using a command you previously relied on. I wanted to try to put forth something possibly useful without breaking stuff.

> You would use the same command, but would be able to press RET to have the default regexp (symbol at point) used as FROM. I don't have a patch yet, but it might be something that all callers of query-replace-read-args might benefit from.

Since I think I can understand what you’re suggesting, I would be happy to make an attempt at a patch to modify the callers of query-replace-read-args to default to a regexp quoted symbol-at-point. But no worries if you would rather have experienced eyes on it.

> 
> And you can actually do this right now:
> 
> - Enter command 'project-query-replace-regexp'
> - Press M-n, having the symbol at point picked up as FROM, hit return
> - Enter the replacement string and hit return

This is awesome! Did not know about that behavior, thank you for sharing!


[-- Attachment #2: Type: text/html, Size: 4546 bytes --]

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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-01-12  3:43     ` Dmitry Gutov
@ 2022-01-12  9:42       ` Jon Eskin
  2022-01-13  1:03         ` Dmitry Gutov
  0 siblings, 1 reply; 42+ messages in thread
From: Jon Eskin @ 2022-01-12  9:42 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Manuel Uberti, emacs-devel



> On Jan 11, 2022, at 10:43 PM, Dmitry Gutov <dgutov@yandex.ru> wrote:
> 
> On 11.01.2022 11:50, Jon Eskin wrote:
>> That's a great function, I love seeing all the results in an xref buffer. I previously explored adding a command to the xref similar to 'xref-query-replace-in-results' that pre-populates the FROM field with the item you were searching for when you executed 'project-find-regexp'.
> 
> Would it help to have an easier version of xref-query-replace-in-results, or do we really need a separate command called project-find-regexp-and-replace?
 
An easier version of xref-query-replace-in-results would be preferable to me personally, at least. 


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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-01-12  3:46   ` Dmitry Gutov
@ 2022-01-12 12:45     ` Eli Zaretskii
  2022-01-13  1:19       ` Dmitry Gutov
  0 siblings, 1 reply; 42+ messages in thread
From: Eli Zaretskii @ 2022-01-12 12:45 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: eskinjp, emacs-devel

> Cc: emacs-devel@gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Wed, 12 Jan 2022 05:46:53 +0200
> 
> On 11.01.2022 15:10, Eli Zaretskii wrote:
> > I think we should have such a command outside project.el as well.
> > Would it be possible to base it on xref.el and related facilities, so
> > that, for example, one could rename a symbol in all the files
> > mentioned in TAGS table?
> 
> We could add a command like xref-find-references-and-replace.

That's what I had in mind.  Perhaps it would even make sense to have
that command automatically adjust itself to a project, when invoked in
the context of a project?

> Or you can do a search with M-? and then press 'r'. Have you tried that?

At some point.  But I think someone who wants refactoring might have
trouble discovering that, and a separate command will make that
easier.



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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-01-12  8:03   ` Jon Eskin
@ 2022-01-12 19:43     ` Dmitry Gutov
  2022-01-12 19:56       ` Juri Linkov
  0 siblings, 1 reply; 42+ messages in thread
From: Dmitry Gutov @ 2022-01-12 19:43 UTC (permalink / raw)
  To: Jon Eskin, Juri Linkov; +Cc: emacs-devel

On 12.01.2022 10:03, Jon Eskin wrote:

>> If we're trying to improve project-query-replace-regexp, why not make 
>> it use the symbol at point by default?
>>
> 
> I thought about the option of using symbol at point by default, but I 
> noticed that the command currently uses the history variable. I wondered 
> if some people were relying on its current behavior in a way that I 
> didn’t anticipate (especially since I’m pretty new to all this). If so, 
> it would be an annoying change to have to worry about whether the cursor 
> is on a symbol when using a command you previously relied on. I wanted 
> to try to put forth something possibly useful without breaking stuff.
> 
>> You would use the same command, but would be able to press RET to have 
>> the default regexp (symbol at point) used as FROM. I don't have a 
>> patch yet, but it might be something that all callers of 
>> query-replace-read-args might benefit from.
> 
> Since I think I can understand what you’re suggesting, I would be happy 
> to make an attempt at a patch to modify the callers of 
> query-replace-read-args to default to a regexp quoted symbol-at-point. 
> But no worries if you would rather have experienced eyes on it.

I'm actually not sure about the best way to implement this: whether we 
definitely want query-replace-read-args to use symbol-at-point as the 
default FROM. But it does sounds handy to me. Let's ask the developer 
who touched it last.

Juri, what do you think?

>> And you can actually do this right now:
>>
>> - Enter command 'project-query-replace-regexp'
>> - Press M-n, having the symbol at point picked up as FROM, hit return
>> - Enter the replacement string and hit return
> 
> This is awesome! Did not know about that behavior, thank you for sharing!

That capability actually comes with the "forward history" thing. Not 
very obvious, but when you learn about it, it's pretty nice.



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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-01-12 19:43     ` Dmitry Gutov
@ 2022-01-12 19:56       ` Juri Linkov
  2022-01-12 22:12         ` Dmitry Gutov
  0 siblings, 1 reply; 42+ messages in thread
From: Juri Linkov @ 2022-01-12 19:56 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Jon Eskin, emacs-devel

>>> If we're trying to improve project-query-replace-regexp, why not make it
>>> use the symbol at point by default?
>>>
>> I thought about the option of using symbol at point by default, but
>> I noticed that the command currently uses the history
>> variable. I wondered if some people were relying on its current behavior
>> in a way that I didn’t anticipate (especially since I’m pretty new to all
>> this). If so, it would be an annoying change to have to worry about
>> whether the cursor is on a symbol when using a command you previously
>> relied on. I wanted to try to put forth something possibly useful without
>> breaking stuff.
>> 
>>> You would use the same command, but would be able to press RET to have
>>> the default regexp (symbol at point) used as FROM. I don't have a patch
>>> yet, but it might be something that all callers of
>>> query-replace-read-args might benefit from.
>> Since I think I can understand what you’re suggesting, I would be happy
>> to make an attempt at a patch to modify the callers of
>> query-replace-read-args to default to a regexp quoted
>> symbol-at-point. But no worries if you would rather have experienced eyes
>> on it.
>
> I'm actually not sure about the best way to implement this: whether we
> definitely want query-replace-read-args to use symbol-at-point as the
> default FROM. But it does sounds handy to me. Let's ask the developer who
> touched it last.

query-replace-read-args can't be changed because in query-replace
RET should use the previous from->to pair from the history.

So if you want, you could change project-query-replace-regexp:
let-bind read-regexp-defaults-function around the call
of query-replace-read-args, and use a symbol at point as the default.

>>> And you can actually do this right now:
>>>
>>> - Enter command 'project-query-replace-regexp'
>>> - Press M-n, having the symbol at point picked up as FROM, hit return
>>> - Enter the replacement string and hit return
>> This is awesome! Did not know about that behavior, thank you for sharing!
>
> That capability actually comes with the "forward history" thing. Not very
> obvious, but when you learn about it, it's pretty nice.

Maybe the prompt should show some indication that the default
can be fetched by M-n?



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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-01-12 19:56       ` Juri Linkov
@ 2022-01-12 22:12         ` Dmitry Gutov
  0 siblings, 0 replies; 42+ messages in thread
From: Dmitry Gutov @ 2022-01-12 22:12 UTC (permalink / raw)
  To: Juri Linkov; +Cc: Jon Eskin, emacs-devel

On 12.01.2022 21:56, Juri Linkov wrote:
>>>> If we're trying to improve project-query-replace-regexp, why not make it
>>>> use the symbol at point by default?
>>>>
>>> I thought about the option of using symbol at point by default, but
>>> I noticed that the command currently uses the history
>>> variable. I wondered if some people were relying on its current behavior
>>> in a way that I didn’t anticipate (especially since I’m pretty new to all
>>> this). If so, it would be an annoying change to have to worry about
>>> whether the cursor is on a symbol when using a command you previously
>>> relied on. I wanted to try to put forth something possibly useful without
>>> breaking stuff.
>>>
>>>> You would use the same command, but would be able to press RET to have
>>>> the default regexp (symbol at point) used as FROM. I don't have a patch
>>>> yet, but it might be something that all callers of
>>>> query-replace-read-args might benefit from.
>>> Since I think I can understand what you’re suggesting, I would be happy
>>> to make an attempt at a patch to modify the callers of
>>> query-replace-read-args to default to a regexp quoted
>>> symbol-at-point. But no worries if you would rather have experienced eyes
>>> on it.
>>
>> I'm actually not sure about the best way to implement this: whether we
>> definitely want query-replace-read-args to use symbol-at-point as the
>> default FROM. But it does sounds handy to me. Let's ask the developer who
>> touched it last.
> 
> query-replace-read-args can't be changed because in query-replace
> RET should use the previous from->to pair from the history.

That probably makes sense. But the user could use 'M-p' for that and 
otherwise enjoy a different default (FROM defaulting to symbol-at-point).

But it would be a breaking change, admittedly.

> So if you want, you could change project-query-replace-regexp:
> let-bind read-regexp-defaults-function around the call
> of query-replace-read-args, and use a symbol at point as the default.

...or that. After all, when doing project-wise replacements, you 
probably wouldn't repeat the same search twice. Or at least, not do it 
often.

>>>> And you can actually do this right now:
>>>>
>>>> - Enter command 'project-query-replace-regexp'
>>>> - Press M-n, having the symbol at point picked up as FROM, hit return
>>>> - Enter the replacement string and hit return
>>> This is awesome! Did not know about that behavior, thank you for sharing!
>>
>> That capability actually comes with the "forward history" thing. Not very
>> obvious, but when you learn about it, it's pretty nice.
> 
> Maybe the prompt should show some indication that the default
> can be fetched by M-n?

Could be an improvement, but I'm not sure how that'd look, or what text 
to show ("future history" is not a very obvious name).



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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-01-12  9:42       ` Jon Eskin
@ 2022-01-13  1:03         ` Dmitry Gutov
  2022-01-13  9:57           ` Jon Eskin
  0 siblings, 1 reply; 42+ messages in thread
From: Dmitry Gutov @ 2022-01-13  1:03 UTC (permalink / raw)
  To: Jon Eskin; +Cc: Manuel Uberti, emacs-devel

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

On 12.01.2022 11:42, Jon Eskin wrote:
> 
> 
>> On Jan 11, 2022, at 10:43 PM, Dmitry Gutov <dgutov@yandex.ru> wrote:
>>
>> On 11.01.2022 11:50, Jon Eskin wrote:
>>> That's a great function, I love seeing all the results in an xref buffer. I previously explored adding a command to the xref similar to 'xref-query-replace-in-results' that pre-populates the FROM field with the item you were searching for when you executed 'project-find-regexp'.
>>
>> Would it help to have an easier version of xref-query-replace-in-results, or do we really need a separate command called project-find-regexp-and-replace?
>   
> An easier version of xref-query-replace-in-results would be preferable to me personally, at least.

Try out this change, then (attached).

[-- Attachment #2: simpler-xref-query-replace-in-results.diff --]
[-- Type: text/x-patch, Size: 1256 bytes --]

diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index 9ce63a8f8a..06faf16a31 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -738,11 +738,20 @@ xref-query-replace-in-results
   "Perform interactive replacement of FROM with TO in all displayed xrefs.
 
 This command interactively replaces FROM with TO in the names of the
-references displayed in the current *xref* buffer."
+references displayed in the current *xref* buffer.
+
+When called interactively, it uses '.*' as FROM, which means
+replace the whole name.  Unless called with prefix argument, in
+which case the user is prompted for both FROM and TO."
   (interactive
-   (let ((fr (read-regexp "Xref query-replace (regexp)" ".*")))
-     (list fr
-           (read-regexp (format "Xref query-replace (regexp) %s with: " fr)))))
+   (let* ((fr
+           (if prefix-arg
+               (read-regexp "Query-replace (regexp)" ".*")
+             ".*"))
+          (prompt (if prefix-arg
+                      (format "Query-replace (regexp) %s with: " fr)
+                    "Query-replace all matches with: ")))
+     (list fr (read-regexp prompt))))
   (let* (item xrefs iter)
     (save-excursion
       (while (setq item (xref--search-property 'xref-item))

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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-01-12 12:45     ` Eli Zaretskii
@ 2022-01-13  1:19       ` Dmitry Gutov
  2022-01-13  8:25         ` Eli Zaretskii
  0 siblings, 1 reply; 42+ messages in thread
From: Dmitry Gutov @ 2022-01-13  1:19 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: eskinjp, emacs-devel

On 12.01.2022 14:45, Eli Zaretskii wrote:
>> Cc: emacs-devel@gnu.org
>> From: Dmitry Gutov <dgutov@yandex.ru>
>> Date: Wed, 12 Jan 2022 05:46:53 +0200
>>
>> On 11.01.2022 15:10, Eli Zaretskii wrote:
>>> I think we should have such a command outside project.el as well.
>>> Would it be possible to base it on xref.el and related facilities, so
>>> that, for example, one could rename a symbol in all the files
>>> mentioned in TAGS table?
>>
>> We could add a command like xref-find-references-and-replace.
> 
> That's what I had in mind.  Perhaps it would even make sense to have
> that command automatically adjust itself to a project, when invoked in
> the context of a project?

The conceptual problem with that is we have a number of commands which 
produce a list of matches in an Xref buffer:

xref-find-references,
xref-find-apropos,
project-find-regexp,
project-or-external-find-regexp

They're all fairly, so I won't think there's a change of implementing 
any meaningful automatic switching based on context.

Do we create the -and-replace counterpart only for xref-find-references?

There's also dired-do-find-regexp for which we have added said 
counterpart already (dired-do-find-regexp-and-replace), but that was 
primarily for backward compatibility of the UI.

>> Or you can do a search with M-? and then press 'r'. Have you tried that?
> 
> At some point.  But I think someone who wants refactoring might have
> trouble discovering that, and a separate command will make that
> easier.

I rather remember the old recommendation to 'M-x find-grep-dired' 
followed by '% m RET .*' and 'M-x dired-do-query-replace-regexp', when 
one wanted to replace across the project. I think we're rather spoiled 
these days by comparison.

Anyway, if you're sure adding xref-find-references-and-replace will 
help, I've got no problem with that.



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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-01-13  1:19       ` Dmitry Gutov
@ 2022-01-13  8:25         ` Eli Zaretskii
  2022-01-14  2:43           ` Dmitry Gutov
  0 siblings, 1 reply; 42+ messages in thread
From: Eli Zaretskii @ 2022-01-13  8:25 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: eskinjp, emacs-devel

> Cc: eskinjp@gmail.com, emacs-devel@gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Thu, 13 Jan 2022 03:19:12 +0200
> 
> The conceptual problem with that is we have a number of commands which 
> produce a list of matches in an Xref buffer:
> 
> xref-find-references,
> xref-find-apropos,
> project-find-regexp,
> project-or-external-find-regexp
> 
> They're all fairly, so I won't think there's a change of implementing 
> any meaningful automatic switching based on context.
> 
> Do we create the -and-replace counterpart only for xref-find-references?

Yes, that's what I wanted to propose.

> There's also dired-do-find-regexp for which we have added said 
> counterpart already (dired-do-find-regexp-and-replace), but that was 
> primarily for backward compatibility of the UI.

Right.

> >> Or you can do a search with M-? and then press 'r'. Have you tried that?
> > 
> > At some point.  But I think someone who wants refactoring might have
> > trouble discovering that, and a separate command will make that
> > easier.
> 
> I rather remember the old recommendation to 'M-x find-grep-dired' 
> followed by '% m RET .*' and 'M-x dired-do-query-replace-regexp', when 
> one wanted to replace across the project. I think we're rather spoiled 
> these days by comparison.
> 
> Anyway, if you're sure adding xref-find-references-and-replace will 
> help, I've got no problem with that.

I think it will be a good addition to what we already have in that
department.  And given that we document it in the user manual, it
should become known to users soon enough.



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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-01-13  1:03         ` Dmitry Gutov
@ 2022-01-13  9:57           ` Jon Eskin
  2022-01-14  2:39             ` Dmitry Gutov
  0 siblings, 1 reply; 42+ messages in thread
From: Jon Eskin @ 2022-01-13  9:57 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Manuel Uberti, emacs-devel

This works great, it's exactly what I was looking to do originally but couldn’t quite figure out how. Thanks for writing it!

> On Jan 12, 2022, at 8:03 PM, Dmitry Gutov <dgutov@yandex.ru> wrote:
> 
> On 12.01.2022 11:42, Jon Eskin wrote:
>>> On Jan 11, 2022, at 10:43 PM, Dmitry Gutov <dgutov@yandex.ru> wrote:
>>> 
>>> On 11.01.2022 11:50, Jon Eskin wrote:
>>>> That's a great function, I love seeing all the results in an xref buffer. I previously explored adding a command to the xref similar to 'xref-query-replace-in-results' that pre-populates the FROM field with the item you were searching for when you executed 'project-find-regexp'.
>>> 
>>> Would it help to have an easier version of xref-query-replace-in-results, or do we really need a separate command called project-find-regexp-and-replace?
>>  An easier version of xref-query-replace-in-results would be preferable to me personally, at least.
> 
> Try out this change, then (attached).
> <simpler-xref-query-replace-in-results.diff>




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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-01-13  9:57           ` Jon Eskin
@ 2022-01-14  2:39             ` Dmitry Gutov
  0 siblings, 0 replies; 42+ messages in thread
From: Dmitry Gutov @ 2022-01-14  2:39 UTC (permalink / raw)
  To: Jon Eskin; +Cc: Manuel Uberti, emacs-devel

On 13.01.2022 11:57, Jon Eskin wrote:
> This works great, it's exactly what I was looking to do originally but couldn’t quite figure out how. Thanks for writing it!

Thanks for testing! I've pushed the change to the master branch.



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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-01-13  8:25         ` Eli Zaretskii
@ 2022-01-14  2:43           ` Dmitry Gutov
  2022-01-14  7:46             ` Eli Zaretskii
  2022-01-14 10:26             ` Jon Eskin
  0 siblings, 2 replies; 42+ messages in thread
From: Dmitry Gutov @ 2022-01-14  2:43 UTC (permalink / raw)
  To: Eli Zaretskii, Jon Eskin; +Cc: emacs-devel

On 13.01.2022 10:25, Eli Zaretskii wrote:

>> Anyway, if you're sure adding xref-find-references-and-replace will
>> help, I've got no problem with that.
> 
> I think it will be a good addition to what we already have in that
> department.  And given that we document it in the user manual, it
> should become known to users soon enough.

I've added that command now.

Though since it also uses query-replace-read-args, it suffers from the 
same (possibly) drawback as project-search and 
project-query-replace-regexp: not using the symbol at point by default.

So Jon, if you fancy writing a patch in this area, you can try 
implementing what Juri suggested:

   let-bind read-regexp-defaults-function around the call
   of query-replace-read-args, and use a symbol at point as the default.

That can apply to both project-query-replace-regexp and 
xref-find-references-and-replace.



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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-01-14  2:43           ` Dmitry Gutov
@ 2022-01-14  7:46             ` Eli Zaretskii
  2022-01-14 10:26             ` Jon Eskin
  1 sibling, 0 replies; 42+ messages in thread
From: Eli Zaretskii @ 2022-01-14  7:46 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: eskinjp, emacs-devel

> Cc: emacs-devel@gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Fri, 14 Jan 2022 04:43:40 +0200
> 
> On 13.01.2022 10:25, Eli Zaretskii wrote:
> 
> >> Anyway, if you're sure adding xref-find-references-and-replace will
> >> help, I've got no problem with that.
> > 
> > I think it will be a good addition to what we already have in that
> > department.  And given that we document it in the user manual, it
> > should become known to users soon enough.
> 
> I've added that command now.

Thanks.



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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-01-14  2:43           ` Dmitry Gutov
  2022-01-14  7:46             ` Eli Zaretskii
@ 2022-01-14 10:26             ` Jon Eskin
  2022-01-14 20:28               ` Dmitry Gutov
  1 sibling, 1 reply; 42+ messages in thread
From: Jon Eskin @ 2022-01-14 10:26 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Eli Zaretskii, emacs-devel


> So Jon, if you fancy writing a patch in this area, you can try implementing what Juri suggested:
> 
>  let-bind read-regexp-defaults-function around the call
>  of query-replace-read-args, and use a symbol at point as the default.
> 
> That can apply to both project-query-replace-regexp and xref-find-references-and-replace.


On it! I might take a little while but I'll check in if I get stuck.


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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-01-14 10:26             ` Jon Eskin
@ 2022-01-14 20:28               ` Dmitry Gutov
  2022-01-15  9:55                 ` Jon Eskin
  0 siblings, 1 reply; 42+ messages in thread
From: Dmitry Gutov @ 2022-01-14 20:28 UTC (permalink / raw)
  To: Jon Eskin; +Cc: Eli Zaretskii, emacs-devel

On 14.01.2022 12:26, Jon Eskin wrote:
>> So Jon, if you fancy writing a patch in this area, you can try implementing what Juri suggested:
>>
>>   let-bind read-regexp-defaults-function around the call
>>   of query-replace-read-args, and use a symbol at point as the default.
>>
>> That can apply to both project-query-replace-regexp and xref-find-references-and-replace.
> 
> On it! I might take a little while but I'll check in if I get stuck.

Yeah, don't hesitate to ask questions if/when you have any.



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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-01-14 20:28               ` Dmitry Gutov
@ 2022-01-15  9:55                 ` Jon Eskin
  2022-01-15 18:30                   ` Juri Linkov
                                     ` (2 more replies)
  0 siblings, 3 replies; 42+ messages in thread
From: Jon Eskin @ 2022-01-15  9:55 UTC (permalink / raw)
  To: Dmitry Gutov, Juri Linkov; +Cc: Eli Zaretskii, emacs-devel

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



> On Jan 14, 2022, at 3:28 PM, Dmitry Gutov <dgutov@yandex.ru> wrote:
> 
> On 14.01.2022 12:26, Jon Eskin wrote:
>>> So Jon, if you fancy writing a patch in this area, you can try implementing what Juri suggested:
>>> 
>>>  let-bind read-regexp-defaults-function around the call
>>>  of query-replace-read-args, and use a symbol at point as the default.
>>> 
>>> That can apply to both project-query-replace-regexp and xref-find-references-and-replace.
>> On it! I might take a little while but I'll check in if I get stuck.
> 
> Yeah, don't hesitate to ask questions if/when you have any.


Hey all,

I took a stab at the implementation Juri described for `project-query-replace-regexp` and I was able to get the correct behavior, but I wanted to check in and see if it needs improvements before looking at `xref-find-references-and-replace.`

When I first let-bound `read-regexp-defaults-function` around the call to `query-replace-read-args`, it didn't work- it looks like `read-regexp` needs to be passed a symbol for its `DEFAULTS` parameter or it ignores `read-regexp-default-function`. I passed in the symbol at point to `DEFAULTS` which works- if I understand correctly the value of any I pass in doesn't end up making a difference as long as it's a symbol. I wasn't sure what the reason was for that behavior, but I didn't want to mess with stuff I didn't understand.

Another issue is that hardcoded logic in `read-regexp`:

'If PROMPT ends in \":\" (followed by
optional whitespace), use it as-is.  Otherwise, add \": \" to the end,
possibly preceded by the default result (see below).'

The PROMPT passed into read-regexp does end in a ":" due to it being formatted by a call to `format-prompt` in `query-replace-read-from`. As a result, when the symbol is at point, `read-regexp` display the prompt with the last replacement from history. To address this, I added a cond case where the formatting takes place and omit the formatting step when there is a symbol at point so that `read-regex` will correctly format the prompt with the symbol at point. 

The result is that a call to `project-query-replace-regexp` will take the symbol at point as the default when available, otherwise it will use its previous behavior of defaulting to the last replacement available in history.

Let me know what you think.


[-- Attachment #2: 0001-Use-symbol-at-point-as-default-for-query-replace-rea.patch --]
[-- Type: application/octet-stream, Size: 3330 bytes --]

From 86c180b8b9021eb96ac2ea2f75682753a3822e50 Mon Sep 17 00:00:00 2001
From: Jon Eskin <eskinjp@gmail.com>
Date: Fri, 14 Jan 2022 10:03:30 -0500
Subject: [PATCH] Use symbol at point as default for query-replace-read-args

* lisp/progmodes/project.el
(project-query-replace-regexp): Let-bind
`read-regexp-defaults-function` to default to symbol at point.

* lisp/replace.el
(query-replace-read-from): Don't format prompt if there's a symbol at
point, otherwise read-regexp is forced to use the wrong prompt.
---
 lisp/progmodes/project.el |  8 +++++---
 lisp/replace.el           | 10 ++++++----
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index c812f28c1b..b44b23554f 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -1072,9 +1072,11 @@ project-query-replace-regexp
 If you exit the `query-replace', you can later continue the
 `query-replace' loop using the command \\[fileloop-continue]."
   (interactive
-   (pcase-let ((`(,from ,to)
-                (query-replace-read-args "Query replace (regexp)" t t)))
-     (list from to)))
+   (let ((read-regexp-defaults-function (lambda ()
+                                          (find-tag-default-as-regexp))))
+    (pcase-let ((`(,from ,to)
+                 (query-replace-read-args "Query replace (regexp)" t t)))
+      (list from to))))
   (fileloop-initialize-replace
    from to (project-files (project-current t)) 'default)
   (fileloop-continue))
diff --git a/lisp/replace.el b/lisp/replace.el
index 60e507c642..3d7648611b 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -208,7 +208,8 @@ query-replace-read-from
 wants to replace FROM with TO."
   (if query-replace-interactive
       (car (if regexp-flag regexp-search-ring search-ring))
-    (let* ((history-add-new-input nil)
+    (let* ((sym-at-point (symbol-at-point))
+           (history-add-new-input nil)
 	   (separator-string
 	    (when query-replace-from-to-separator
 	      ;; Check if the first non-whitespace char is displayable
@@ -234,8 +235,9 @@ query-replace-read-from
 	     (symbol-value query-replace-from-history-variable)))
 	   (minibuffer-allow-text-properties t) ; separator uses text-properties
 	   (prompt
-	    (cond ((and query-replace-defaults separator)
-                   (format-prompt prompt (car minibuffer-history)))
+	    (cond (sym-at-point prompt) ;; if there's a symbol at point, let read-regexp format the prompt
+                  ((and query-replace-defaults separator)
+                     (format-prompt prompt (car minibuffer-history)))
                   (query-replace-defaults
                    (format-prompt
                     prompt (format "%s -> %s"
@@ -255,7 +257,7 @@ query-replace-read-from
                                 (append '((separator . t) (face . t))
                                         text-property-default-nonsticky)))
                 (if regexp-flag
-                    (read-regexp prompt nil 'minibuffer-history)
+                    (read-regexp prompt sym-at-point 'minibuffer-history)
                   (read-from-minibuffer
                    prompt nil nil nil nil
                    (query-replace-read-from-suggestions) t)))))
-- 
2.34.1


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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-01-15  9:55                 ` Jon Eskin
@ 2022-01-15 18:30                   ` Juri Linkov
  2022-01-16  3:02                     ` Dmitry Gutov
  2022-01-15 18:41                   ` Jon Eskin
  2022-01-17  1:41                   ` Dmitry Gutov
  2 siblings, 1 reply; 42+ messages in thread
From: Juri Linkov @ 2022-01-15 18:30 UTC (permalink / raw)
  To: Jon Eskin; +Cc: Eli Zaretskii, emacs-devel, Dmitry Gutov

> When I first let-bound `read-regexp-defaults-function` around the call to
> `query-replace-read-args`, it didn't work- it looks like `read-regexp`
> needs to be passed a symbol for its `DEFAULTS` parameter or it ignores
> `read-regexp-default-function`. I passed in the symbol at point to
> `DEFAULTS` which works- if I understand correctly the value of any I pass
> in doesn't end up making a difference as long as it's a symbol. I wasn't
> sure what the reason was for that behavior, but I didn't want to mess with
> stuff I didn't understand.

This is needed to handle `read-regexp` in `occur-read-primary-args`
that uses the symbol `regexp-history-last` by default.

> Another issue is that hardcoded logic in `read-regexp`:
>
> 'If PROMPT ends in \":\" (followed by
> optional whitespace), use it as-is.  Otherwise, add \": \" to the end,
> possibly preceded by the default result (see below).'
>
> The PROMPT passed into read-regexp does end in a ":" due to it being
> formatted by a call to `format-prompt` in `query-replace-read-from`. As
> a result, when the symbol is at point, `read-regexp` display the prompt
> with the last replacement from history. To address this, I added a cond
> case where the formatting takes place and omit the formatting step when
> there is a symbol at point so that `read-regex` will correctly format the
> prompt with the symbol at point.
>
> The result is that a call to `project-query-replace-regexp` will take
> the symbol at point as the default when available, otherwise it will
> use its previous behavior of defaulting to the last replacement
> available in history.
>
> Let me know what you think.

Before finishing this implementation, please answer one question.
`query-replace-read-from` uses two minibuffer-reading functions:

                (if regexp-flag
                    (read-regexp prompt sym-at-point 'minibuffer-history)
                  (read-from-minibuffer
                   prompt nil nil nil nil
                   (query-replace-read-from-suggestions) t))

Do you think the same default with the symbol at point
should be used for the non-regexp case with read-from-minibuffer as well?
If yes, then we need a different solution that works for both cases.



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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-01-15  9:55                 ` Jon Eskin
  2022-01-15 18:30                   ` Juri Linkov
@ 2022-01-15 18:41                   ` Jon Eskin
  2022-01-16 17:55                     ` Juri Linkov
  2022-01-17  1:41                   ` Dmitry Gutov
  2 siblings, 1 reply; 42+ messages in thread
From: Jon Eskin @ 2022-01-15 18:41 UTC (permalink / raw)
  To: Dmitry Gutov, Juri Linkov; +Cc: Eli Zaretskii, emacs-devel

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

Apologies- in the event that this patch is satisfactory, the following includes `xref-find-references-and-replace`. I didn’t realize it would be a simple replacement. 

[-- Attachment #2: 0001-Use-symbol-at-point-as-default-for-query-replace-rea.patch --]
[-- Type: application/octet-stream, Size: 4126 bytes --]

From 3d575a12107daa09fdb0efc0397ff8c7affa9b1a Mon Sep 17 00:00:00 2001
From: Jon Eskin <eskinjp@gmail.com>
Date: Fri, 14 Jan 2022 10:03:30 -0500
Subject: [PATCH] Use symbol at point as default for query-replace-read-args

* lisp/progmodes/project.el
(project-query-replace-regexp): Let-bind
`read-regexp-defaults-function` to default to symbol at point.

* lisp/replace.el
(query-replace-read-from): Don't format prompt if there's a symbol at
point, otherwise read-regexp is forced to use the wrong prompt.

* lisp/progmodex/xref.el (xref-find-references): Let-bind
`read-regexp-defaults-function` to default to symbol at point
---
 lisp/progmodes/project.el |  8 +++++---
 lisp/progmodes/xref.el    |  4 +++-
 lisp/replace.el           | 10 ++++++----
 3 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index c812f28c1b..b44b23554f 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -1072,9 +1072,11 @@ project-query-replace-regexp
 If you exit the `query-replace', you can later continue the
 `query-replace' loop using the command \\[fileloop-continue]."
   (interactive
-   (pcase-let ((`(,from ,to)
-                (query-replace-read-args "Query replace (regexp)" t t)))
-     (list from to)))
+   (let ((read-regexp-defaults-function (lambda ()
+                                          (find-tag-default-as-regexp))))
+    (pcase-let ((`(,from ,to)
+                 (query-replace-read-args "Query replace (regexp)" t t)))
+      (list from to))))
   (fileloop-initialize-replace
    from to (project-files (project-current t)) 'default)
   (fileloop-continue))
diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index a1e976cb1c..5336c319bb 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -1481,7 +1481,9 @@ xref-find-references
 (defun xref-find-references-and-replace (from to)
   "Replace all references to identifier FROM with TO."
   (interactive
-   (let ((common
+   (let* ((common
+          (read-regexp-defaults-function (lambda ()
+                                          (find-tag-default-as-regexp)))
           (query-replace-read-args "Query replace identifier" nil)))
      (list (nth 0 common) (nth 1 common))))
   (require 'xref)
diff --git a/lisp/replace.el b/lisp/replace.el
index 60e507c642..3d7648611b 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -208,7 +208,8 @@ query-replace-read-from
 wants to replace FROM with TO."
   (if query-replace-interactive
       (car (if regexp-flag regexp-search-ring search-ring))
-    (let* ((history-add-new-input nil)
+    (let* ((sym-at-point (symbol-at-point))
+           (history-add-new-input nil)
 	   (separator-string
 	    (when query-replace-from-to-separator
 	      ;; Check if the first non-whitespace char is displayable
@@ -234,8 +235,9 @@ query-replace-read-from
 	     (symbol-value query-replace-from-history-variable)))
 	   (minibuffer-allow-text-properties t) ; separator uses text-properties
 	   (prompt
-	    (cond ((and query-replace-defaults separator)
-                   (format-prompt prompt (car minibuffer-history)))
+	    (cond (sym-at-point prompt) ;; if there's a symbol at point, let read-regexp format the prompt
+                  ((and query-replace-defaults separator)
+                     (format-prompt prompt (car minibuffer-history)))
                   (query-replace-defaults
                    (format-prompt
                     prompt (format "%s -> %s"
@@ -255,7 +257,7 @@ query-replace-read-from
                                 (append '((separator . t) (face . t))
                                         text-property-default-nonsticky)))
                 (if regexp-flag
-                    (read-regexp prompt nil 'minibuffer-history)
+                    (read-regexp prompt sym-at-point 'minibuffer-history)
                   (read-from-minibuffer
                    prompt nil nil nil nil
                    (query-replace-read-from-suggestions) t)))))
-- 
2.34.1


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



> On Jan 15, 2022, at 4:55 AM, Jon Eskin <eskinjp@gmail.com> wrote:
> 
> 
> 
>> On Jan 14, 2022, at 3:28 PM, Dmitry Gutov <dgutov@yandex.ru> wrote:
>> 
>> On 14.01.2022 12:26, Jon Eskin wrote:
>>>> So Jon, if you fancy writing a patch in this area, you can try implementing what Juri suggested:
>>>> 
>>>> let-bind read-regexp-defaults-function around the call
>>>> of query-replace-read-args, and use a symbol at point as the default.
>>>> 
>>>> That can apply to both project-query-replace-regexp and xref-find-references-and-replace.
>>> On it! I might take a little while but I'll check in if I get stuck.
>> 
>> Yeah, don't hesitate to ask questions if/when you have any.
> 
> 
> Hey all,
> 
> I took a stab at the implementation Juri described for `project-query-replace-regexp` and I was able to get the correct behavior, but I wanted to check in and see if it needs improvements before looking at `xref-find-references-and-replace.`
> 
> When I first let-bound `read-regexp-defaults-function` around the call to `query-replace-read-args`, it didn't work- it looks like `read-regexp` needs to be passed a symbol for its `DEFAULTS` parameter or it ignores `read-regexp-default-function`. I passed in the symbol at point to `DEFAULTS` which works- if I understand correctly the value of any I pass in doesn't end up making a difference as long as it's a symbol. I wasn't sure what the reason was for that behavior, but I didn't want to mess with stuff I didn't understand.
> 
> Another issue is that hardcoded logic in `read-regexp`:
> 
> 'If PROMPT ends in \":\" (followed by
> optional whitespace), use it as-is.  Otherwise, add \": \" to the end,
> possibly preceded by the default result (see below).'
> 
> The PROMPT passed into read-regexp does end in a ":" due to it being formatted by a call to `format-prompt` in `query-replace-read-from`. As a result, when the symbol is at point, `read-regexp` display the prompt with the last replacement from history. To address this, I added a cond case where the formatting takes place and omit the formatting step when there is a symbol at point so that `read-regex` will correctly format the prompt with the symbol at point. 
> 
> The result is that a call to `project-query-replace-regexp` will take the symbol at point as the default when available, otherwise it will use its previous behavior of defaulting to the last replacement available in history.
> 
> Let me know what you think.
> 
> <0001-Use-symbol-at-point-as-default-for-query-replace-rea.patch>


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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-01-15 18:30                   ` Juri Linkov
@ 2022-01-16  3:02                     ` Dmitry Gutov
  2022-01-16 17:58                       ` Juri Linkov
  0 siblings, 1 reply; 42+ messages in thread
From: Dmitry Gutov @ 2022-01-16  3:02 UTC (permalink / raw)
  To: Juri Linkov, Jon Eskin; +Cc: Eli Zaretskii, emacs-devel

On 15.01.2022 20:30, Juri Linkov wrote:
>> When I first let-bound `read-regexp-defaults-function` around the call to
>> `query-replace-read-args`, it didn't work- it looks like `read-regexp`
>> needs to be passed a symbol for its `DEFAULTS` parameter or it ignores
>> `read-regexp-default-function`. I passed in the symbol at point to
>> `DEFAULTS` which works- if I understand correctly the value of any I pass
>> in doesn't end up making a difference as long as it's a symbol. I wasn't
>> sure what the reason was for that behavior, but I didn't want to mess with
>> stuff I didn't understand.
> 
> This is needed to handle `read-regexp` in `occur-read-primary-args`
> that uses the symbol `regexp-history-last` by default.

occur?

read-regexp is called from query-replace-read-from.

Binding read-regexp-defaults-function doesn't seem to work because its 
use is for some reason predicated on (and defaults (symbolp defaults)) 
evaluating to non-nil.

And 'read-regexp' is called with nil second argument.

> Before finishing this implementation, please answer one question.
> `query-replace-read-from` uses two minibuffer-reading functions:
> 
>                  (if regexp-flag
>                      (read-regexp prompt sym-at-point 'minibuffer-history)
>                    (read-from-minibuffer
>                     prompt nil nil nil nil
>                     (query-replace-read-from-suggestions) t))
> 
> Do you think the same default with the symbol at point
> should be used for the non-regexp case with read-from-minibuffer as well?
> If yes, then we need a different solution that works for both cases.

It doesn't have to be the same. project-query-replace-regexp could bind 
read-regexp-defaults-function, and xref-find-references-and-replace 
could bind something else. But that variable still needs to be created.



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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-01-15 18:41                   ` Jon Eskin
@ 2022-01-16 17:55                     ` Juri Linkov
  0 siblings, 0 replies; 42+ messages in thread
From: Juri Linkov @ 2022-01-16 17:55 UTC (permalink / raw)
  To: Jon Eskin; +Cc: Eli Zaretskii, emacs-devel, Dmitry Gutov

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

> Apologies- in the event that this patch is satisfactory, the following
> includes `xref-find-references-and-replace`.  I didn’t realize it
> would be a simple replacement.
> [...]
> @@ -1072,9 +1072,11 @@ project-query-replace-regexp
> -   (pcase-let ((`(,from ,to)
> -                (query-replace-read-args "Query replace (regexp)" t t)))
> -     (list from to)))
> +   (let ((read-regexp-defaults-function (lambda ()
> +                                          (find-tag-default-as-regexp))))
> +    (pcase-let ((`(,from ,to)
> +                 (query-replace-read-args "Query replace (regexp)" t t)))
> +      (list from to))))

The problem was in let-binding 'read-regexp-defaults-function' to a lambda.
It should work when using 'find-tag-default-as-regexp' as a symbol:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: read-regexp-defaults-function.patch --]
[-- Type: text/x-diff, Size: 2778 bytes --]

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index c812f28c1b..c58c05bc71 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -1072,9 +1072,10 @@ project-query-replace-regexp
 If you exit the `query-replace', you can later continue the
 `query-replace' loop using the command \\[fileloop-continue]."
   (interactive
-   (pcase-let ((`(,from ,to)
-                (query-replace-read-args "Query replace (regexp)" t t)))
-     (list from to)))
+   (let ((read-regexp-defaults-function 'find-tag-default-as-regexp))
+     (pcase-let ((`(,from ,to)
+                  (query-replace-read-args "Query replace (regexp)" t t)))
+       (list from to))))
   (fileloop-initialize-replace
    from to (project-files (project-current t)) 'default)
   (fileloop-continue))
diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index 37e2159782..640c8745db 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -1481,8 +1481,9 @@ xref-find-references
 (defun xref-find-references-and-replace (from to)
   "Replace all references to identifier FROM with TO."
   (interactive
-   (let ((common
-          (query-replace-read-args "Query replace identifier" nil)))
+   (let* ((read-regexp-defaults-function 'find-tag-default-as-regexp)
+          (common
+           (query-replace-read-args "Query replace identifier" nil)))
      (list (nth 0 common) (nth 1 common))))
   (require 'xref)
   (with-current-buffer
diff --git a/lisp/replace.el b/lisp/replace.el
index 60e507c642..9c7680a563 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -234,7 +234,8 @@ query-replace-read-from
 	     (symbol-value query-replace-from-history-variable)))
 	   (minibuffer-allow-text-properties t) ; separator uses text-properties
 	   (prompt
-	    (cond ((and query-replace-defaults separator)
+            (cond ((and regexp-flag read-regexp-defaults-function) prompt)
+                  ((and query-replace-defaults separator)
                    (format-prompt prompt (car minibuffer-history)))
                   (query-replace-defaults
                    (format-prompt
@@ -255,7 +256,10 @@ query-replace-read-from
                                 (append '((separator . t) (face . t))
                                         text-property-default-nonsticky)))
                 (if regexp-flag
-                    (read-regexp prompt nil 'minibuffer-history)
+                    (read-regexp
+                     (if read-regexp-defaults-function (string-remove-suffix ": " prompt))
+                     read-regexp-defaults-function
+                     'minibuffer-history)
                   (read-from-minibuffer
                    prompt nil nil nil nil
                    (query-replace-read-from-suggestions) t)))))

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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-01-16  3:02                     ` Dmitry Gutov
@ 2022-01-16 17:58                       ` Juri Linkov
  2022-01-17  0:19                         ` Dmitry Gutov
  0 siblings, 1 reply; 42+ messages in thread
From: Juri Linkov @ 2022-01-16 17:58 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Jon Eskin, Eli Zaretskii, emacs-devel

>> This is needed to handle `read-regexp` in `occur-read-primary-args`
>> that uses the symbol `regexp-history-last` by default.
>
> occur?
>
> read-regexp is called from query-replace-read-from.
>
> Binding read-regexp-defaults-function doesn't seem to work because its use
> is for some reason predicated on (and defaults (symbolp defaults))
> evaluating to non-nil.
>
> And 'read-regexp' is called with nil second argument.

The customizable user option 'read-regexp-defaults-function' was created
for the following problem: by default, 'occur' uses the last history item
as the default value, so 'read-regexp-defaults-function' could be
customized to get a tag at point as the default value of 'occur'.

Exactly the same situation is with 'query-replace-read-from':
by default, it uses the last history item, so changing the value of
'read-regexp-defaults-function' should also affect the query-replace
like in the patch that I sent to Jon.

>> Before finishing this implementation, please answer one question.
>> `query-replace-read-from` uses two minibuffer-reading functions:
>>                  (if regexp-flag
>>                      (read-regexp prompt sym-at-point 'minibuffer-history)
>>                    (read-from-minibuffer
>>                     prompt nil nil nil nil
>>                     (query-replace-read-from-suggestions) t))
>> Do you think the same default with the symbol at point
>> should be used for the non-regexp case with read-from-minibuffer as well?
>> If yes, then we need a different solution that works for both cases.
>
> It doesn't have to be the same. project-query-replace-regexp could bind
> read-regexp-defaults-function, and xref-find-references-and-replace could
> bind something else. But that variable still needs to be created.

xref-find-references-and-replace calls query-replace-read-args
with a non-regexp flag, so I have no idea what new variable
could be created for read-from-minibuffer.  It already uses
'query-replace-read-from-suggestions' where the top value
could be the default.



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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-01-16 17:58                       ` Juri Linkov
@ 2022-01-17  0:19                         ` Dmitry Gutov
  2022-01-17  1:15                           ` Dmitry Gutov
  0 siblings, 1 reply; 42+ messages in thread
From: Dmitry Gutov @ 2022-01-17  0:19 UTC (permalink / raw)
  To: Juri Linkov; +Cc: Jon Eskin, Eli Zaretskii, emacs-devel

On 16.01.2022 19:58, Juri Linkov wrote:
>>> This is needed to handle `read-regexp` in `occur-read-primary-args`
>>> that uses the symbol `regexp-history-last` by default.
>>
>> occur?
>>
>> read-regexp is called from query-replace-read-from.
>>
>> Binding read-regexp-defaults-function doesn't seem to work because its use
>> is for some reason predicated on (and defaults (symbolp defaults))
>> evaluating to non-nil.
>>
>> And 'read-regexp' is called with nil second argument.
> 
> The customizable user option 'read-regexp-defaults-function' was created
> for the following problem: by default, 'occur' uses the last history item
> as the default value, so 'read-regexp-defaults-function' could be
> customized to get a tag at point as the default value of 'occur'.
> 
> Exactly the same situation is with 'query-replace-read-from':
> by default, it uses the last history item, so changing the value of
> 'read-regexp-defaults-function' should also affect the query-replace
> like in the patch that I sent to Jon.

The patch works for project-query-replace-regexp but not for 
xref-find-references-and-replace, which still doesn't suggest the symbol 
at point as FROM (BTW, in the final version this function should 
probably use #'find-tag-default rather than 'find-tag-default-as-regexp).

>>> Before finishing this implementation, please answer one question.
>>> `query-replace-read-from` uses two minibuffer-reading functions:
>>>                   (if regexp-flag
>>>                       (read-regexp prompt sym-at-point 'minibuffer-history)
>>>                     (read-from-minibuffer
>>>                      prompt nil nil nil nil
>>>                      (query-replace-read-from-suggestions) t))
>>> Do you think the same default with the symbol at point
>>> should be used for the non-regexp case with read-from-minibuffer as well?
>>> If yes, then we need a different solution that works for both cases.
>>
>> It doesn't have to be the same. project-query-replace-regexp could bind
>> read-regexp-defaults-function, and xref-find-references-and-replace could
>> bind something else. But that variable still needs to be created.
> 
> xref-find-references-and-replace calls query-replace-read-args
> with a non-regexp flag, so I have no idea what new variable
> could be created for read-from-minibuffer.

I don't really have a preference as to how the implementation is 
organized, as long as it all works.

As far as I'm concerned, it might as well use 
read-regexp-defaults-function. But the implementation is made more 
complicated by the fact that read-regexp and read-from-minibuffer treat 
their PROMPT and DEFAULTS arguments differently.

E.g. the latter won't return DEFAULT-VALUE on empty input and won't 
format the prompt. If I use read-string instead, it uses the default 
value instead of empty input, but does not format the prompt still.

> It already uses
> 'query-replace-read-from-suggestions' where the top value
> could be the default.

That would affect all callers of query-replace-read-from, wouldn't it? 
I'm not sure, but a conservative approach (which we often take here) 
would be to try to avoid that.



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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-01-17  0:19                         ` Dmitry Gutov
@ 2022-01-17  1:15                           ` Dmitry Gutov
  2022-01-17  8:17                             ` Juri Linkov
  0 siblings, 1 reply; 42+ messages in thread
From: Dmitry Gutov @ 2022-01-17  1:15 UTC (permalink / raw)
  To: Juri Linkov; +Cc: Jon Eskin, Eli Zaretskii, emacs-devel

On 17.01.2022 02:19, Dmitry Gutov wrote:
> As far as I'm concerned, it might as well use 
> read-regexp-defaults-function. But the implementation is made more 
> complicated by the fact that read-regexp and read-from-minibuffer treat 
> their PROMPT and DEFAULTS arguments differently.
> 
> E.g. the latter won't return DEFAULT-VALUE on empty input and won't 
> format the prompt. If I use read-string instead, it uses the default 
> value instead of empty input, but does not format the prompt still.

So, this seems to work, but I wonder if it's the best we can do:

diff --git a/lisp/replace.el b/lisp/replace.el
index 60e507c642..9847e4ebea 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -234,7 +234,10 @@ query-replace-read-from
  	     (symbol-value query-replace-from-history-variable)))
  	   (minibuffer-allow-text-properties t) ; separator uses text-properties
  	   (prompt
-	    (cond ((and query-replace-defaults separator)
+            (cond ((and regexp-flag read-regexp-defaults-function) prompt)
+                  (read-regexp-defaults-function
+                   (format-prompt prompt (funcall 
read-regexp-defaults-function)))
+                  ((and query-replace-defaults separator)
                     (format-prompt prompt (car minibuffer-history)))
                    (query-replace-defaults
                     (format-prompt
@@ -255,10 +258,17 @@ query-replace-read-from
                                  (append '((separator . t) (face . t))
                                          text-property-default-nonsticky)))
                  (if regexp-flag
-                    (read-regexp prompt nil 'minibuffer-history)
-                  (read-from-minibuffer
-                   prompt nil nil nil nil
-                   (query-replace-read-from-suggestions) t)))))
+                    (read-regexp
+                     (if read-regexp-defaults-function 
(string-remove-suffix ": " prompt))
+                     read-regexp-defaults-function
+                     'minibuffer-history)
+                  (read-string
+                   prompt
+                   nil nil
+                   (if read-regexp-defaults-function
+                       (funcall read-regexp-defaults-function)
+                     (query-replace-read-from-suggestions))
+                   t)))))
             (to))
        (if (and (zerop (length from)) query-replace-defaults)
  	  (cons (caar query-replace-defaults)



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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-01-15  9:55                 ` Jon Eskin
  2022-01-15 18:30                   ` Juri Linkov
  2022-01-15 18:41                   ` Jon Eskin
@ 2022-01-17  1:41                   ` Dmitry Gutov
  2022-01-17  6:36                     ` Jon Eskin
  2 siblings, 1 reply; 42+ messages in thread
From: Dmitry Gutov @ 2022-01-17  1:41 UTC (permalink / raw)
  To: Jon Eskin, Juri Linkov; +Cc: Eli Zaretskii, emacs-devel

Jon,

Thanks for taking a stab at this, by the way.

To clarify why your patch was not enough:

On 15.01.2022 11:55, Jon Eskin wrote:
> When I first let-bound `read-regexp-defaults-function` around the call to `query-replace-read-args`, it didn't work- it looks like `read-regexp` needs to be passed a symbol for its `DEFAULTS` parameter or it ignores `read-regexp-default-function`. I passed in the symbol at point to `DEFAULTS` which works- if I understand correctly the value of any I pass in doesn't end up making a difference as long as it's a symbol. I wasn't sure what the reason was for that behavior, but I didn't want to mess with stuff I didn't understand.

This change (having query-replace-read-from use symbol-at-point) affects 
all its callers, whereas the idea was to delegate the default detection 
logic to a piece of code determined by the caller.

That's why the next step was to determine why binding 
'read-regexp-defaults-function' didn't have the desired effect, rather 
than calling 'symbol-at-point' directly.



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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-01-17  1:41                   ` Dmitry Gutov
@ 2022-01-17  6:36                     ` Jon Eskin
  0 siblings, 0 replies; 42+ messages in thread
From: Jon Eskin @ 2022-01-17  6:36 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Eli Zaretskii, emacs-devel, Juri Linkov

No problem! Thank you for helping walk me through this along the way, I really appreciate it and learned a lot.

> On Jan 16, 2022, at 8:41 PM, Dmitry Gutov <dgutov@yandex.ru> wrote:
> 
> Jon,
> 
> Thanks for taking a stab at this, by the way.
> 
> To clarify why your patch was not enough:
> 
> On 15.01.2022 11:55, Jon Eskin wrote:
>> When I first let-bound `read-regexp-defaults-function` around the call to `query-replace-read-args`, it didn't work- it looks like `read-regexp` needs to be passed a symbol for its `DEFAULTS` parameter or it ignores `read-regexp-default-function`. I passed in the symbol at point to `DEFAULTS` which works- if I understand correctly the value of any I pass in doesn't end up making a difference as long as it's a symbol. I wasn't sure what the reason was for that behavior, but I didn't want to mess with stuff I didn't understand.
> 
> This change (having query-replace-read-from use symbol-at-point) affects all its callers, whereas the idea was to delegate the default detection logic to a piece of code determined by the caller.
> 
> That's why the next step was to determine why binding 'read-regexp-defaults-function' didn't have the desired effect, rather than calling 'symbol-at-point' directly.




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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-01-17  1:15                           ` Dmitry Gutov
@ 2022-01-17  8:17                             ` Juri Linkov
  2022-01-21  3:06                               ` Dmitry Gutov
  0 siblings, 1 reply; 42+ messages in thread
From: Juri Linkov @ 2022-01-17  8:17 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Jon Eskin, Eli Zaretskii, emacs-devel

> +                  (read-string
> +                   prompt
> +                   nil nil
> +                   (if read-regexp-defaults-function
> +                       (funcall read-regexp-defaults-function)
> +                     (query-replace-read-from-suggestions))

Isn't it too strange that read-string uses the read-regexp function?
Maybe better to add a new variable read-string-defaults-function.
Then it could provide an option to use 'car' of
query-replace-read-from-suggestions, and allow a custom function.



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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-01-17  8:17                             ` Juri Linkov
@ 2022-01-21  3:06                               ` Dmitry Gutov
  2022-01-31 18:25                                 ` Juri Linkov
  0 siblings, 1 reply; 42+ messages in thread
From: Dmitry Gutov @ 2022-01-21  3:06 UTC (permalink / raw)
  To: Juri Linkov; +Cc: Jon Eskin, Eli Zaretskii, emacs-devel

Hi Juri,

On 17.01.2022 10:17, Juri Linkov wrote:
>> +                  (read-string
>> +                   prompt
>> +                   nil nil
>> +                   (if read-regexp-defaults-function
>> +                       (funcall read-regexp-defaults-function)
>> +                     (query-replace-read-from-suggestions))
> Isn't it too strange that read-string uses the read-regexp function?
> Maybe better to add a new variable read-string-defaults-function.

That sounds fine to me.

> Then it could provide an option to use 'car' of
> query-replace-read-from-suggestions, and allow a custom function.

...less confident about this one, because read-regexp-defaults-function 
doesn't do anything like that, right? It might be nice if the options 
mirrored each other.

But if it's too much of a bother right now, I could just add two 
hand-written prompts with default to xref-find-references-and-replace, 
without the fancy "arrow". It's not like the replacements history is 
going to see too much use for this command.

In the meantime, I think you should install your latest patch now 
(without the xref.el part, since it doesn't work yet).



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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-01-21  3:06                               ` Dmitry Gutov
@ 2022-01-31 18:25                                 ` Juri Linkov
  2022-02-01  2:10                                   ` Dmitry Gutov
  0 siblings, 1 reply; 42+ messages in thread
From: Juri Linkov @ 2022-01-31 18:25 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Jon Eskin, Eli Zaretskii, emacs-devel

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

>>> +                  (read-string
>>> +                   prompt
>>> +                   nil nil
>>> +                   (if read-regexp-defaults-function
>>> +                       (funcall read-regexp-defaults-function)
>>> +                     (query-replace-read-from-suggestions))
>> Isn't it too strange that read-string uses the read-regexp function?
>> Maybe better to add a new variable read-string-defaults-function.
>
> That sounds fine to me.
>
>> Then it could provide an option to use 'car' of
>> query-replace-read-from-suggestions, and allow a custom function.
>
> ...less confident about this one, because read-regexp-defaults-function
> doesn't do anything like that, right? It might be nice if the options
> mirrored each other.
>
> But if it's too much of a bother right now, I could just add two
> hand-written prompts with default to xref-find-references-and-replace,
> without the fancy "arrow". It's not like the replacements history is going
> to see too much use for this command.
>
> In the meantime, I think you should install your latest patch now (without
> the xref.el part, since it doesn't work yet).

After thinking more about this, I can't find a possible use for
read-string-defaults-function, because every call of read-string
provides own default value.  Also using read-regexp-defaults-function
in query-replace-read-from is not the right thing either - when
the users already customized it for e.g. occur, it would be too
unexpected when it will use a tag at point instead of from->to
pairs in query-replace.

Since query-replace is a very special command, the most uncontroversial
thing to do for a conservative approach would be to add two specific
variables (that later could be turned into options when needed):


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: query-replace-read-from-default.patch --]
[-- Type: text/x-diff, Size: 2515 bytes --]

diff --git a/lisp/replace.el b/lisp/replace.el
index 689a3f77ed..91cd2b3420 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -186,6 +186,12 @@ query-replace--split-string
                         length)
              length)))))
 
+(defvar query-replace-read-from-default nil
+  "Function to get default non-regexp value for `query-replace-read-from'.")
+
+(defvar query-replace-read-from-regexp-default nil
+  "Function to get default regexp value for `query-replace-read-from'.")
+
 (defun query-replace-read-from-suggestions ()
   "Return a list of standard suggestions for `query-replace-read-from'.
 By default, the list includes the active region, the identifier
@@ -234,7 +240,10 @@ query-replace-read-from
 	     (symbol-value query-replace-from-history-variable)))
 	   (minibuffer-allow-text-properties t) ; separator uses text-properties
 	   (prompt
-	    (cond ((and query-replace-defaults separator)
+            (cond ((and query-replace-read-from-regexp-default regexp-flag) prompt)
+                  ((and query-replace-read-from-default (not regexp-flag))
+                   (format-prompt prompt (funcall query-replace-read-from-default)))
+                  ((and query-replace-defaults separator)
                    (format-prompt prompt (car minibuffer-history)))
                   (query-replace-defaults
                    (format-prompt
@@ -255,10 +264,19 @@ query-replace-read-from
                                 (append '((separator . t) (face . t))
                                         text-property-default-nonsticky)))
                 (if regexp-flag
-                    (read-regexp prompt nil 'minibuffer-history)
+                    (read-regexp
+                     (if query-replace-read-from-regexp-default
+                         (string-remove-suffix ": " prompt)
+                       prompt)
+                     query-replace-read-from-regexp-default
+                     'minibuffer-history)
                   (read-from-minibuffer
                    prompt nil nil nil nil
-                   (query-replace-read-from-suggestions) t)))))
+                   (if query-replace-read-from-default
+                       (cons (funcall query-replace-read-from-default)
+                             (query-replace-read-from-suggestions))
+                     (query-replace-read-from-suggestions))
+                   t)))))
            (to))
       (if (and (zerop (length from)) query-replace-defaults)
 	  (cons (caar query-replace-defaults)

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


Then you could use these variables like this:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: query-replace-read-from-regexp-default.patch --]
[-- Type: text/x-diff, Size: 1465 bytes --]

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index c812f28c1b..f606a25575 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -1072,9 +1072,10 @@ project-query-replace-regexp
 If you exit the `query-replace', you can later continue the
 `query-replace' loop using the command \\[fileloop-continue]."
   (interactive
-   (pcase-let ((`(,from ,to)
-                (query-replace-read-args "Query replace (regexp)" t t)))
-     (list from to)))
+   (let ((query-replace-read-from-regexp-default 'find-tag-default-as-regexp))
+     (pcase-let ((`(,from ,to)
+                  (query-replace-read-args "Query replace (regexp)" t t)))
+       (list from to))))
   (fileloop-initialize-replace
    from to (project-files (project-current t)) 'default)
   (fileloop-continue))
diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index 37e2159782..4efa652084 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -1481,8 +1481,9 @@ xref-find-references
 (defun xref-find-references-and-replace (from to)
   "Replace all references to identifier FROM with TO."
   (interactive
-   (let ((common
-          (query-replace-read-args "Query replace identifier" nil)))
+   (let* ((query-replace-read-from-default 'find-tag-default)
+          (common
+           (query-replace-read-args "Query replace identifier" nil)))
      (list (nth 0 common) (nth 1 common))))
   (require 'xref)
   (with-current-buffer

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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-01-31 18:25                                 ` Juri Linkov
@ 2022-02-01  2:10                                   ` Dmitry Gutov
  2022-02-01 20:09                                     ` Juri Linkov
  0 siblings, 1 reply; 42+ messages in thread
From: Dmitry Gutov @ 2022-02-01  2:10 UTC (permalink / raw)
  To: Juri Linkov; +Cc: Jon Eskin, Eli Zaretskii, emacs-devel

Hi Juri,

On 31.01.2022 20:25, Juri Linkov wrote:
> After thinking more about this, I can't find a possible use for
> read-string-defaults-function, because every call of read-string
> provides own default value.  Also using read-regexp-defaults-function
> in query-replace-read-from is not the right thing either - when
> the users already customized it for e.g. occur, it would be too
> unexpected when it will use a tag at point instead of from->to
> pairs in query-replace.
> 
> Since query-replace is a very special command, the most uncontroversial
> thing to do for a conservative approach would be to add two specific
> variables (that later could be turned into options when needed):

Sounds reasonable, thanks.

If you're satisfied with the change, please go ahead and install the 
patches.



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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-02-01  2:10                                   ` Dmitry Gutov
@ 2022-02-01 20:09                                     ` Juri Linkov
  2022-02-01 22:07                                       ` Dmitry Gutov
  0 siblings, 1 reply; 42+ messages in thread
From: Juri Linkov @ 2022-02-01 20:09 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Jon Eskin, Eli Zaretskii, emacs-devel

>> After thinking more about this, I can't find a possible use for
>> read-string-defaults-function, because every call of read-string
>> provides own default value.  Also using read-regexp-defaults-function
>> in query-replace-read-from is not the right thing either - when
>> the users already customized it for e.g. occur, it would be too
>> unexpected when it will use a tag at point instead of from->to
>> pairs in query-replace.
>> Since query-replace is a very special command, the most uncontroversial
>> thing to do for a conservative approach would be to add two specific
>> variables (that later could be turned into options when needed):
>
> Sounds reasonable, thanks.
>
> If you're satisfied with the change, please go ahead and install the
> patches.

So now patches are installed in master.



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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-02-01 20:09                                     ` Juri Linkov
@ 2022-02-01 22:07                                       ` Dmitry Gutov
  2022-02-02 19:51                                         ` Juri Linkov
  0 siblings, 1 reply; 42+ messages in thread
From: Dmitry Gutov @ 2022-02-01 22:07 UTC (permalink / raw)
  To: Juri Linkov; +Cc: Jon Eskin, Eli Zaretskii, emacs-devel

On 01.02.2022 22:09, Juri Linkov wrote:
>>> After thinking more about this, I can't find a possible use for
>>> read-string-defaults-function, because every call of read-string
>>> provides own default value.  Also using read-regexp-defaults-function
>>> in query-replace-read-from is not the right thing either - when
>>> the users already customized it for e.g. occur, it would be too
>>> unexpected when it will use a tag at point instead of from->to
>>> pairs in query-replace.
>>> Since query-replace is a very special command, the most uncontroversial
>>> thing to do for a conservative approach would be to add two specific
>>> variables (that later could be turned into options when needed):
>> Sounds reasonable, thanks.
>>
>> If you're satisfied with the change, please go ahead and install the
>> patches.
> So now patches are installed in master.

Thanks!

Could you try testing the new behavior in 
xref-find-references-and-replace? I'm seeing this:

- If there are no entries in history, I see the appropriate text in the 
first prompt (featuring the tag near point), but when I press RET, the 
next prompt which should mention the thing to replace, just has two 
spaces in a row. And indeed, the behavior is as if it's read an empty 
string.

- If there are history entries, and I press RET (to use the default 
FROM), it proceeds to do a search right away instead of prompting me for 
TO. And it uses the from-to pair from history instead of the current input.

project-query-replace-regexp seems all right, OTOH.



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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-02-01 22:07                                       ` Dmitry Gutov
@ 2022-02-02 19:51                                         ` Juri Linkov
  2022-02-02 21:09                                           ` Dmitry Gutov
  0 siblings, 1 reply; 42+ messages in thread
From: Juri Linkov @ 2022-02-02 19:51 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Jon Eskin, Eli Zaretskii, emacs-devel

> On 01.02.2022 22:09, Juri Linkov wrote:
>>>> After thinking more about this, I can't find a possible use for
>>>> read-string-defaults-function, because every call of read-string
>>>> provides own default value.  Also using read-regexp-defaults-function
>>>> in query-replace-read-from is not the right thing either - when
>>>> the users already customized it for e.g. occur, it would be too
>>>> unexpected when it will use a tag at point instead of from->to
>>>> pairs in query-replace.
>>>> Since query-replace is a very special command, the most uncontroversial
>>>> thing to do for a conservative approach would be to add two specific
>>>> variables (that later could be turned into options when needed):
>>> Sounds reasonable, thanks.
>>>
>>> If you're satisfied with the change, please go ahead and install the
>>> patches.
>> So now patches are installed in master.
>
> Thanks!
>
> Could you try testing the new behavior in xref-find-references-and-replace?

Sorry, I tested everything in the previous version of the patch,
but then forgot that read-string is different from read-from-minibuffer.

> I'm seeing this:
>
> - If there are no entries in history, I see the appropriate text in the
>   first prompt (featuring the tag near point), but when I press RET, the
>   next prompt which should mention the thing to replace, just has two
>   spaces in a row. And indeed, the behavior is as if it's read an empty
>   string.
>
> - If there are history entries, and I press RET (to use the default FROM),
>   it proceeds to do a search right away instead of prompting me for TO. And
>  it uses the from-to pair from history instead of the current input.

Now everything is fixed (at least in all tested cases).



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

* Re: [Patch] Add project.el command to replace symbol at point throughout project
  2022-02-02 19:51                                         ` Juri Linkov
@ 2022-02-02 21:09                                           ` Dmitry Gutov
  0 siblings, 0 replies; 42+ messages in thread
From: Dmitry Gutov @ 2022-02-02 21:09 UTC (permalink / raw)
  To: Juri Linkov; +Cc: Jon Eskin, Eli Zaretskii, emacs-devel

On 02.02.2022 21:51, Juri Linkov wrote:
>> On 01.02.2022 22:09, Juri Linkov wrote:
>>>>> After thinking more about this, I can't find a possible use for
>>>>> read-string-defaults-function, because every call of read-string
>>>>> provides own default value.  Also using read-regexp-defaults-function
>>>>> in query-replace-read-from is not the right thing either - when
>>>>> the users already customized it for e.g. occur, it would be too
>>>>> unexpected when it will use a tag at point instead of from->to
>>>>> pairs in query-replace.
>>>>> Since query-replace is a very special command, the most uncontroversial
>>>>> thing to do for a conservative approach would be to add two specific
>>>>> variables (that later could be turned into options when needed):
>>>> Sounds reasonable, thanks.
>>>>
>>>> If you're satisfied with the change, please go ahead and install the
>>>> patches.
>>> So now patches are installed in master.
>>
>> Thanks!
>>
>> Could you try testing the new behavior in xref-find-references-and-replace?
> 
> Sorry, I tested everything in the previous version of the patch,
> but then forgot that read-string is different from read-from-minibuffer.
> 
>> I'm seeing this:
>>
>> - If there are no entries in history, I see the appropriate text in the
>>    first prompt (featuring the tag near point), but when I press RET, the
>>    next prompt which should mention the thing to replace, just has two
>>    spaces in a row. And indeed, the behavior is as if it's read an empty
>>    string.
>>
>> - If there are history entries, and I press RET (to use the default FROM),
>>    it proceeds to do a search right away instead of prompting me for TO. And
>>   it uses the from-to pair from history instead of the current input.
> 
> Now everything is fixed (at least in all tested cases).

Looking good now. Thanks!



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

end of thread, other threads:[~2022-02-02 21:09 UTC | newest]

Thread overview: 42+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-01-11  7:45 [Patch] Add project.el command to replace symbol at point throughout project Jon Eskin
2022-01-11  7:51 ` Manuel Uberti
2022-01-11  9:50   ` Jon Eskin
2022-01-12  3:43     ` Dmitry Gutov
2022-01-12  9:42       ` Jon Eskin
2022-01-13  1:03         ` Dmitry Gutov
2022-01-13  9:57           ` Jon Eskin
2022-01-14  2:39             ` Dmitry Gutov
2022-01-11 13:10 ` Eli Zaretskii
2022-01-11 15:15   ` Daniel Martín
2022-01-11 17:17     ` Eli Zaretskii
2022-01-12  3:46   ` Dmitry Gutov
2022-01-12 12:45     ` Eli Zaretskii
2022-01-13  1:19       ` Dmitry Gutov
2022-01-13  8:25         ` Eli Zaretskii
2022-01-14  2:43           ` Dmitry Gutov
2022-01-14  7:46             ` Eli Zaretskii
2022-01-14 10:26             ` Jon Eskin
2022-01-14 20:28               ` Dmitry Gutov
2022-01-15  9:55                 ` Jon Eskin
2022-01-15 18:30                   ` Juri Linkov
2022-01-16  3:02                     ` Dmitry Gutov
2022-01-16 17:58                       ` Juri Linkov
2022-01-17  0:19                         ` Dmitry Gutov
2022-01-17  1:15                           ` Dmitry Gutov
2022-01-17  8:17                             ` Juri Linkov
2022-01-21  3:06                               ` Dmitry Gutov
2022-01-31 18:25                                 ` Juri Linkov
2022-02-01  2:10                                   ` Dmitry Gutov
2022-02-01 20:09                                     ` Juri Linkov
2022-02-01 22:07                                       ` Dmitry Gutov
2022-02-02 19:51                                         ` Juri Linkov
2022-02-02 21:09                                           ` Dmitry Gutov
2022-01-15 18:41                   ` Jon Eskin
2022-01-16 17:55                     ` Juri Linkov
2022-01-17  1:41                   ` Dmitry Gutov
2022-01-17  6:36                     ` Jon Eskin
2022-01-12  3:42 ` Dmitry Gutov
2022-01-12  8:03   ` Jon Eskin
2022-01-12 19:43     ` Dmitry Gutov
2022-01-12 19:56       ` Juri Linkov
2022-01-12 22:12         ` 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).