unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / Atom feed
* bug#45617: <query-replace> loses the edit region. Works in 23.3, broke in 26.3
@ 2021-01-03  1:05 Bob Floyd
  2021-01-03 14:47 ` Eli Zaretskii
  0 siblings, 1 reply; 19+ messages in thread
From: Bob Floyd @ 2021-01-03  1:05 UTC (permalink / raw)
  To: 45617

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

Use this c++ test:

main()

{

  int         _xx;

 

  _xx = _xx + 2;

 

  return _xx;

}

 

"query-replace" loses the edit region. 

Works in Emacs 23.3. Broke in GNU Emacs 26.3 (build 1, x86_64-w64-mingw32)

of 2019-08-29

 

1. Select region from line 2 '{' to line 8 '}'

 

2. Do "query-replace" to begin "Query replace in region"

 

3. Do Double-click left mouse to select '_xx' on line 3

   -or-

   <down-mouse-one> "mouse-drag-region" to select '_xx' on line 3

 

4. In "Query replace in region:" window do <mouse-2> "mouse-yank-at-click"

   to insert '_xx'.

 

5. Via keyboard hit "Enter" key, now you see:

   "Query replace in region _xx with:"

 

6. Via keyboard type '_yy' and 'enter'

"Query replace in region _xx with: _yy"

 

7. Nothing happens because the region in step (1) has been lost.

    It should ask to replace 4 '_xx's in the region.


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

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

* bug#45617: <query-replace> loses the edit region. Works in 23.3, broke in 26.3
  2021-01-03  1:05 bug#45617: <query-replace> loses the edit region. Works in 23.3, broke in 26.3 Bob Floyd
@ 2021-01-03 14:47 ` Eli Zaretskii
  2021-01-03 19:18   ` Bob Floyd
  2021-01-04 17:37   ` Juri Linkov
  0 siblings, 2 replies; 19+ messages in thread
From: Eli Zaretskii @ 2021-01-03 14:47 UTC (permalink / raw)
  To: Bob Floyd; +Cc: 45617

> From: "Bob Floyd" <bobfloyd@comcast.net>
> Date: Sat, 2 Jan 2021 17:05:57 -0800
> 
> 7. Nothing happens because the region in step (1) has been lost.
>     It should ask to replace 4 ‘_xx’s in the region.

I suspect this is due to changes in how selection works in Emacs.
Those changes were made in Emacs 24; please see the section "Selection
changes" in the file etc/NEWS.24, where you will also find
instructions for getting back the old behavior.





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

* bug#45617: <query-replace> loses the edit region. Works in 23.3, broke in 26.3
  2021-01-03 14:47 ` Eli Zaretskii
@ 2021-01-03 19:18   ` Bob Floyd
  2021-01-04 17:26     ` Eli Zaretskii
  2021-01-04 17:37   ` Juri Linkov
  1 sibling, 1 reply; 19+ messages in thread
From: Bob Floyd @ 2021-01-03 19:18 UTC (permalink / raw)
  To: 'Eli Zaretskii'; +Cc: 45617

Hi Eli,

I already had the NEWS.24 changes in my .emacs file! So that is not the cause of this issue.

You do agree this is a bug - that <query-replace> fails, correct?

Thanks for your quick reply.
Bob

-----Original Message-----
From: Eli Zaretskii [mailto:eliz@gnu.org] 
Sent: Sunday, January 3, 2021 6:48 AM
To: Bob Floyd
Cc: 45617@debbugs.gnu.org
Subject: Re: bug#45617: <query-replace> loses the edit region. Works in 23.3, broke in 26.3

> From: "Bob Floyd" <bobfloyd@comcast.net>
> Date: Sat, 2 Jan 2021 17:05:57 -0800
> 
> 7. Nothing happens because the region in step (1) has been lost.
>     It should ask to replace 4 �_xx�s in the region.

I suspect this is due to changes in how selection works in Emacs.
Those changes were made in Emacs 24; please see the section "Selection
changes" in the file etc/NEWS.24, where you will also find
instructions for getting back the old behavior.






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

* bug#45617: <query-replace> loses the edit region. Works in 23.3, broke in 26.3
  2021-01-03 19:18   ` Bob Floyd
@ 2021-01-04 17:26     ` Eli Zaretskii
  0 siblings, 0 replies; 19+ messages in thread
From: Eli Zaretskii @ 2021-01-04 17:26 UTC (permalink / raw)
  To: Bob Floyd; +Cc: 45617

> From: "Bob Floyd" <bobfloyd@comcast.net>
> Cc: <45617@debbugs.gnu.org>
> Date: Sun, 3 Jan 2021 11:18:03 -0800
> 
> I already had the NEWS.24 changes in my .emacs file! So that is not the cause of this issue.

That is strange, because if I make all the changes mentioned in
NEWS.24, in "emacs -Q", then your recipe works for me as you expected.

Did you try that in "emacs -Q"?  If not, please try; if "emacs -Q"
with the changes from NEWS.24 does work, then there's some other
customization in your init files which gets in the way.

> You do agree this is a bug - that <query-replace> fails, correct?

It is not a bug, it is a consequence of the changes in how selections
work in Emacs.  Your example assumes the old handling of selections,
so it doesn't work with Emacs >= 24 defaults.





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

* bug#45617: <query-replace> loses the edit region. Works in 23.3, broke in 26.3
  2021-01-03 14:47 ` Eli Zaretskii
  2021-01-03 19:18   ` Bob Floyd
@ 2021-01-04 17:37   ` Juri Linkov
  2021-01-04 22:40     ` Bob Floyd
  1 sibling, 1 reply; 19+ messages in thread
From: Juri Linkov @ 2021-01-04 17:37 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Bob Floyd, 45617

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

>> 7. Nothing happens because the region in step (1) has been lost.
>>     It should ask to replace 4 ‘_xx’s in the region.
>
> I suspect this is due to changes in how selection works in Emacs.
> Those changes were made in Emacs 24; please see the section "Selection
> changes" in the file etc/NEWS.24, where you will also find
> instructions for getting back the old behavior.

I tried to fix this problem by this patch, and indeed it fixed it,
so the region boundaries are preserved even when changed during
the minibuffer reading the strings:


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

diff --git a/lisp/replace.el b/lisp/replace.el
index 9765b2b5be..c5acf8fca7 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -395,7 +395,10 @@ query-replace
 
 To customize possible responses, change the bindings in `query-replace-map'."
   (interactive
-   (let ((common
+   (let ((start (if (use-region-p) (region-beginning)))
+         (end (if (use-region-p) (region-end)))
+         (region-noncontiguous-p (if (use-region-p) (region-noncontiguous-p)))
+         (common
 	  (query-replace-read-args
 	   (concat "Query replace"
 		   (if current-prefix-arg
@@ -407,10 +410,7 @@ query-replace
 	   ;; These are done separately here
 	   ;; so that command-history will record these expressions
 	   ;; rather than the values they had this time.
-	   (if (use-region-p) (region-beginning))
-	   (if (use-region-p) (region-end))
-	   (nth 3 common)
-	   (if (use-region-p) (region-noncontiguous-p)))))
+	   start end (nth 3 common) region-noncontiguous-p)))
   (perform-replace from-string to-string t nil delimited nil nil start end backward region-noncontiguous-p))
 
 (define-key esc-map "%" 'query-replace)

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


But then I noticed this comment in 'query-replace':

	   ;; These are done separately here
	   ;; so that command-history will record these expressions
	   ;; rather than the values they had this time.

And indeed this patch broke this feature, so region boundaries
are saved as numbers in command-history for repeat-complex-command
instead of such code:

  C-x M-: (query-replace "a" "z" nil (if (use-region-p) (region-beginning)) (if (use-region-p) (region-end)) nil nil)

But anyway this feature was broken by design and never worked:
with a compiled replace.el it saves region boundaries as numbers,
and only when 'query-replace' is manually evaluated with 'eval-defun',
only then region boundaries are saved as code to command-history.

So we could just delete 'fix_command' from callint.c,
and don't worry about such cases.

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

* bug#45617: <query-replace> loses the edit region. Works in 23.3, broke in 26.3
  2021-01-04 17:37   ` Juri Linkov
@ 2021-01-04 22:40     ` Bob Floyd
  2021-01-05 18:27       ` Juri Linkov
  0 siblings, 1 reply; 19+ messages in thread
From: Bob Floyd @ 2021-01-04 22:40 UTC (permalink / raw)
  To: 'Juri Linkov', 'Eli Zaretskii'; +Cc: 45617

Thanks! The patch has fixed the issue. 


-----Original Message-----
From: Juri Linkov [mailto:juri@linkov.net] 
Sent: Monday, January 4, 2021 9:37 AM
To: Eli Zaretskii
Cc: Bob Floyd; 45617@debbugs.gnu.org
Subject: Re: bug#45617: <query-replace> loses the edit region. Works in 23.3, broke in 26.3

>> 7. Nothing happens because the region in step (1) has been lost.
>>     It should ask to replace 4  _xx s in the region.
>
> I suspect this is due to changes in how selection works in Emacs.
> Those changes were made in Emacs 24; please see the section "Selection 
> changes" in the file etc/NEWS.24, where you will also find 
> instructions for getting back the old behavior.

I tried to fix this problem by this patch, and indeed it fixed it, so the region boundaries are preserved even when changed during the minibuffer reading the strings:







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

* bug#45617: <query-replace> loses the edit region. Works in 23.3, broke in 26.3
  2021-01-04 22:40     ` Bob Floyd
@ 2021-01-05 18:27       ` Juri Linkov
  2021-01-05 19:45         ` Bob Floyd
  0 siblings, 1 reply; 19+ messages in thread
From: Juri Linkov @ 2021-01-05 18:27 UTC (permalink / raw)
  To: Bob Floyd; +Cc: 45617

> Thanks! The patch has fixed the issue.

If there is no need to support the broken feature of saving
region boundaries as code in the command history, then
this patch could be pushed.  Otherwise, another solution is
to restore the previously selected region after exiting the minibuffer
the same way as exiting the minibuffer restores window configurations
(there is a new option to disable this discussed in bug#45072).





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

* bug#45617: <query-replace> loses the edit region. Works in 23.3, broke in 26.3
  2021-01-05 18:27       ` Juri Linkov
@ 2021-01-05 19:45         ` Bob Floyd
  2021-01-06 17:44           ` Juri Linkov
  0 siblings, 1 reply; 19+ messages in thread
From: Bob Floyd @ 2021-01-05 19:45 UTC (permalink / raw)
  To: 'Juri Linkov'; +Cc: 45617

For my two cents, what you write sounds like the real problem is the
minibuffer failing to restore on exit. A fix to that would be preferable to
the patch. After all, who knows what else may affected by the minibuffer!
I'd be happy to test this alternative.

-----Original Message-----
From: Juri Linkov [mailto:juri@linkov.net] 
Sent: Tuesday, January 5, 2021 10:27 AM
To: Bob Floyd
Cc: 'Eli Zaretskii'; 45617@debbugs.gnu.org
Subject: Re: bug#45617: <query-replace> loses the edit region. Works in
23.3, broke in 26.3

> Thanks! The patch has fixed the issue.

If there is no need to support the broken feature of saving
region boundaries as code in the command history, then
this patch could be pushed.  Otherwise, another solution is
to restore the previously selected region after exiting the minibuffer
the same way as exiting the minibuffer restores window configurations
(there is a new option to disable this discussed in bug#45072).






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

* bug#45617: <query-replace> loses the edit region. Works in 23.3, broke in 26.3
  2021-01-05 19:45         ` Bob Floyd
@ 2021-01-06 17:44           ` Juri Linkov
  2021-01-06 18:19             ` Eli Zaretskii
  0 siblings, 1 reply; 19+ messages in thread
From: Juri Linkov @ 2021-01-06 17:44 UTC (permalink / raw)
  To: Bob Floyd; +Cc: 45617

> For my two cents, what you write sounds like the real problem is the
> minibuffer failing to restore on exit. A fix to that would be preferable to
> the patch. After all, who knows what else may affected by the minibuffer!
> I'd be happy to test this alternative.

The question we need to answer: should this fix affect all other uses
of the minibuffer?  So for any command that operates on the active region
and asks its arguments from the minibuffer, when you copy the text
from the original buffer to the minibuffer and thus change its region,
should quitting the minibuffer restore the original region for
all such commands?  This means restoring the original mark and point.





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

* bug#45617: <query-replace> loses the edit region. Works in 23.3, broke in 26.3
  2021-01-06 17:44           ` Juri Linkov
@ 2021-01-06 18:19             ` Eli Zaretskii
  2021-01-06 22:10               ` Bob Floyd
  2021-01-07 18:46               ` Juri Linkov
  0 siblings, 2 replies; 19+ messages in thread
From: Eli Zaretskii @ 2021-01-06 18:19 UTC (permalink / raw)
  To: Juri Linkov; +Cc: bobfloyd, 45617

> From: Juri Linkov <juri@linkov.net>
> Cc: "'Eli Zaretskii'" <eliz@gnu.org>,  <45617@debbugs.gnu.org>
> Date: Wed, 06 Jan 2021 19:44:30 +0200
> 
> > For my two cents, what you write sounds like the real problem is the
> > minibuffer failing to restore on exit. A fix to that would be preferable to
> > the patch. After all, who knows what else may affected by the minibuffer!
> > I'd be happy to test this alternative.
> 
> The question we need to answer: should this fix affect all other uses
> of the minibuffer?  So for any command that operates on the active region
> and asks its arguments from the minibuffer, when you copy the text
> from the original buffer to the minibuffer and thus change its region,
> should quitting the minibuffer restore the original region for
> all such commands?  This means restoring the original mark and point.

I think the answer is YES, especially if we did behave like that in
the distant past.





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

* bug#45617: <query-replace> loses the edit region. Works in 23.3, broke in 26.3
  2021-01-06 18:19             ` Eli Zaretskii
@ 2021-01-06 22:10               ` Bob Floyd
  2021-01-13 18:53                 ` Juri Linkov
  2021-01-07 18:46               ` Juri Linkov
  1 sibling, 1 reply; 19+ messages in thread
From: Bob Floyd @ 2021-01-06 22:10 UTC (permalink / raw)
  To: 'Eli Zaretskii', 'Juri Linkov'; +Cc: 45617

Perhaps another way to think about this is that emacs is "mode-less".

I can begin a command, recurse into another, pop out and resume the first
command.

Without the patch, or minibuffer fix, <query-replace> breaks that design.

If the minibffer fix better supports mode-less operation in general it would
be better than the patch.

-----Original Message-----
From: Eli Zaretskii [mailto:eliz@gnu.org] 
Sent: Wednesday, January 6, 2021 10:19 AM
To: Juri Linkov
Cc: bobfloyd@comcast.net; 45617@debbugs.gnu.org
Subject: Re: bug#45617: <query-replace> loses the edit region. Works in
23.3, broke in 26.3

> From: Juri Linkov <juri@linkov.net>
> Cc: "'Eli Zaretskii'" <eliz@gnu.org>,  <45617@debbugs.gnu.org>
> Date: Wed, 06 Jan 2021 19:44:30 +0200
> 
> > For my two cents, what you write sounds like the real problem is the
> > minibuffer failing to restore on exit. A fix to that would be preferable
to
> > the patch. After all, who knows what else may affected by the
minibuffer!
> > I'd be happy to test this alternative.
> 
> The question we need to answer: should this fix affect all other uses
> of the minibuffer?  So for any command that operates on the active region
> and asks its arguments from the minibuffer, when you copy the text
> from the original buffer to the minibuffer and thus change its region,
> should quitting the minibuffer restore the original region for
> all such commands?  This means restoring the original mark and point.

I think the answer is YES, especially if we did behave like that in
the distant past.






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

* bug#45617: <query-replace> loses the edit region. Works in 23.3, broke in 26.3
  2021-01-06 18:19             ` Eli Zaretskii
  2021-01-06 22:10               ` Bob Floyd
@ 2021-01-07 18:46               ` Juri Linkov
  2021-01-07 19:36                 ` Eli Zaretskii
  1 sibling, 1 reply; 19+ messages in thread
From: Juri Linkov @ 2021-01-07 18:46 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: bobfloyd, 45617

>> The question we need to answer: should this fix affect all other uses
>> of the minibuffer?  So for any command that operates on the active region
>> and asks its arguments from the minibuffer, when you copy the text
>> from the original buffer to the minibuffer and thus change its region,
>> should quitting the minibuffer restore the original region for
>> all such commands?  This means restoring the original mark and point.
>
> I think the answer is YES, especially if we did behave like that in
> the distant past.

Interesting, this means this behavior was changed intentionally
in a recent version?





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

* bug#45617: <query-replace> loses the edit region. Works in 23.3, broke in 26.3
  2021-01-07 18:46               ` Juri Linkov
@ 2021-01-07 19:36                 ` Eli Zaretskii
  0 siblings, 0 replies; 19+ messages in thread
From: Eli Zaretskii @ 2021-01-07 19:36 UTC (permalink / raw)
  To: Juri Linkov; +Cc: bobfloyd, 45617

> From: Juri Linkov <juri@linkov.net>
> Cc: bobfloyd@comcast.net,  45617@debbugs.gnu.org
> Date: Thu, 07 Jan 2021 20:46:05 +0200
> 
> >> The question we need to answer: should this fix affect all other uses
> >> of the minibuffer?  So for any command that operates on the active region
> >> and asks its arguments from the minibuffer, when you copy the text
> >> from the original buffer to the minibuffer and thus change its region,
> >> should quitting the minibuffer restore the original region for
> >> all such commands?  This means restoring the original mark and point.
> >
> > I think the answer is YES, especially if we did behave like that in
> > the distant past.
> 
> Interesting, this means this behavior was changed intentionally
> in a recent version?

AFAICT, it did change, but only slightly.





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

* bug#45617: <query-replace> loses the edit region. Works in 23.3, broke in 26.3
  2021-01-06 22:10               ` Bob Floyd
@ 2021-01-13 18:53                 ` Juri Linkov
  2021-01-15  1:22                   ` Bob Floyd
  2021-01-15 18:09                   ` Bob Floyd
  0 siblings, 2 replies; 19+ messages in thread
From: Juri Linkov @ 2021-01-13 18:53 UTC (permalink / raw)
  To: Bob Floyd; +Cc: 45617

> Perhaps another way to think about this is that emacs is "mode-less".
>
> I can begin a command, recurse into another, pop out and resume the first
> command.
>
> Without the patch, or minibuffer fix, <query-replace> breaks that design.

So here is a simpler patch that fixes all query-replace commands:

diff --git a/lisp/replace.el b/lisp/replace.el
index d41dc98a0d..16b80a8fd1 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -327,6 +336,7 @@ query-replace-read-to
 (defun query-replace-read-args (prompt regexp-flag &optional noerror)
   (unless noerror
     (barf-if-buffer-read-only))
+  (save-mark-and-excursion
   (let* ((from (query-replace-read-from prompt regexp-flag))
 	 (to (if (consp from) (prog1 (cdr from) (setq from (car from)))
 	       (query-replace-read-to from prompt regexp-flag))))
@@ -334,7 +344,7 @@ query-replace-read-args
 	  (or (and current-prefix-arg (not (eq current-prefix-arg '-)))
               (and (plist-member (text-properties-at 0 from) 'isearch-regexp-function)
                    (get-text-property 0 'isearch-regexp-function from)))
-	  (and current-prefix-arg (eq current-prefix-arg '-)))))
+	  (and current-prefix-arg (eq current-prefix-arg '-))))))
 
 (defun query-replace (from-string to-string &optional delimited start end backward region-noncontiguous-p)
   "Replace some occurrences of FROM-STRING with TO-STRING.





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

* bug#45617: <query-replace> loses the edit region. Works in 23.3, broke in 26.3
  2021-01-13 18:53                 ` Juri Linkov
@ 2021-01-15  1:22                   ` Bob Floyd
  2021-01-15  8:54                     ` Juri Linkov
  2021-01-15 18:09                   ` Bob Floyd
  1 sibling, 1 reply; 19+ messages in thread
From: Bob Floyd @ 2021-01-15  1:22 UTC (permalink / raw)
  To: 'Juri Linkov'; +Cc: 45617

I've removed the earlier patch and installed these changes. Now testing it!
Thanks.

-----Original Message-----
From: Juri Linkov [mailto:juri@linkov.net] 
Sent: Wednesday, January 13, 2021 10:54 AM
To: Bob Floyd
Cc: 'Eli Zaretskii'; 45617@debbugs.gnu.org
Subject: Re: bug#45617: <query-replace> loses the edit region. Works in
23.3, broke in 26.3

> Perhaps another way to think about this is that emacs is "mode-less".
>
> I can begin a command, recurse into another, pop out and resume the first
> command.
>
> Without the patch, or minibuffer fix, <query-replace> breaks that design.

So here is a simpler patch that fixes all query-replace commands:

diff --git a/lisp/replace.el b/lisp/replace.el
index d41dc98a0d..16b80a8fd1 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -327,6 +336,7 @@ query-replace-read-to
 (defun query-replace-read-args (prompt regexp-flag &optional noerror)
   (unless noerror
     (barf-if-buffer-read-only))
+  (save-mark-and-excursion
   (let* ((from (query-replace-read-from prompt regexp-flag))
 	 (to (if (consp from) (prog1 (cdr from) (setq from (car from)))
 	       (query-replace-read-to from prompt regexp-flag))))
@@ -334,7 +344,7 @@ query-replace-read-args
 	  (or (and current-prefix-arg (not (eq current-prefix-arg '-)))
               (and (plist-member (text-properties-at 0 from)
'isearch-regexp-function)
                    (get-text-property 0 'isearch-regexp-function from)))
-	  (and current-prefix-arg (eq current-prefix-arg '-)))))
+	  (and current-prefix-arg (eq current-prefix-arg '-))))))
 
 (defun query-replace (from-string to-string &optional delimited start end
backward region-noncontiguous-p)
   "Replace some occurrences of FROM-STRING with TO-STRING.






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

* bug#45617: <query-replace> loses the edit region. Works in 23.3, broke in 26.3
  2021-01-15  1:22                   ` Bob Floyd
@ 2021-01-15  8:54                     ` Juri Linkov
  2021-01-15 11:23                       ` Eli Zaretskii
  2021-01-15 18:05                       ` Bob Floyd
  0 siblings, 2 replies; 19+ messages in thread
From: Juri Linkov @ 2021-01-15  8:54 UTC (permalink / raw)
  To: Bob Floyd; +Cc: 45617

> I've removed the earlier patch and installed these changes. Now testing it!
> Thanks.
>
> -----Original Message-----
>  (defun query-replace-read-args (prompt regexp-flag &optional noerror)
>    (unless noerror
>      (barf-if-buffer-read-only))
> +  (save-mark-and-excursion
>    (let* ((from (query-replace-read-from prompt regexp-flag))
>  	 (to (if (consp from) (prog1 (cdr from) (setq from (car from)))
>  	       (query-replace-read-to from prompt regexp-flag))))

It seems this is what we need to push to be able to close this bug report.
Indeed, it fixes only query-replace commands.  But fixing all commands
that use the minibuffer is not as easy as adding 'save-mark-and-excursion'
like in the patch above.

The problem is that 'read-from-minibuffer' is implemented in C.
If it was implemented in Lisp, it would be easy to add just
'save-mark-and-excursion', but in C this is impossible.





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

* bug#45617: <query-replace> loses the edit region. Works in 23.3, broke in 26.3
  2021-01-15  8:54                     ` Juri Linkov
@ 2021-01-15 11:23                       ` Eli Zaretskii
  2021-01-15 18:05                       ` Bob Floyd
  1 sibling, 0 replies; 19+ messages in thread
From: Eli Zaretskii @ 2021-01-15 11:23 UTC (permalink / raw)
  To: Juri Linkov; +Cc: bobfloyd, 45617

> From: Juri Linkov <juri@linkov.net>
> Cc: "'Eli Zaretskii'" <eliz@gnu.org>,  <45617@debbugs.gnu.org>
> Date: Fri, 15 Jan 2021 10:54:56 +0200
> 
> The problem is that 'read-from-minibuffer' is implemented in C.
> If it was implemented in Lisp, it would be easy to add just
> 'save-mark-and-excursion', but in C this is impossible.

You can do the equivalent of that in C, can't you?





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

* bug#45617: <query-replace> loses the edit region. Works in 23.3, broke in 26.3
  2021-01-15  8:54                     ` Juri Linkov
  2021-01-15 11:23                       ` Eli Zaretskii
@ 2021-01-15 18:05                       ` Bob Floyd
  1 sibling, 0 replies; 19+ messages in thread
From: Bob Floyd @ 2021-01-15 18:05 UTC (permalink / raw)
  To: 'Juri Linkov'; +Cc: 45617

I'm having an issue with the new patch. <query-replace> fails!

1. Select entire region by dragging mouse.
2. Begin <query-replace>
   In the command window: Query replace in region: 
3. Enter RL using the keyboard
   In the command window: Query replace in region RL with: 
4. Now, with the mouse, select RL at line 12 ...
   paramset P2 RL; <--- THIS ONE
   ... then yank it to the command window and with keyboard enter X:
   In the command window: Query replace in region RL with: RLX
5. Enter
   Only one of the two RL's in the region are selected, depending on
   if the region was selected from top-to-bottom or bottom-to-top.

   It should select both RL's!

----------------------------------------------------
module RL ( electrical in, out );
   parameter real R1 = 7000.0;
   parameter real C  = 1p;
   
   analog begin
      I(in,out) <+ V(in,out) / R1;
      I(out)    <+ C * ddt( V(out) );
   end
   
endmodule // Sub

paramset P2 RL;
parameter real R=1000;
.R1= R;
endparamset
----------------------------------------------------

Just in case I misread the diff, this is the function as I have manually
patched it:

(defun query-replace-read-args (prompt regexp-flag &optional noerror)
  (unless noerror
    (barf-if-buffer-read-only))
  (save-mark-and-excursion
  (let* ((from (query-replace-read-from prompt regexp-flag))
	 (to (if (consp from) (prog1 (cdr from) (setq from (car from)))
	       (query-replace-read-to from prompt regexp-flag))))
    (or (and current-prefix-arg (not (eq current-prefix-arg '-)))
	(and (plist-member (text-properties-at 0 from)
'isearch-regexp-function)
(get-text-property 0 'isearch-regexp-function from)))
    (list from to
	  (and current-prefix-arg (not (eq current-prefix-arg '-)))
	  (and current-prefix-arg (eq current-prefix-arg '-))))))



-----Original Message-----
From: Juri Linkov [mailto:juri@linkov.net] 
Sent: Friday, January 15, 2021 12:55 AM
To: Bob Floyd
Cc: 'Eli Zaretskii'; 45617@debbugs.gnu.org
Subject: Re: bug#45617: <query-replace> loses the edit region. Works in
23.3, broke in 26.3

> I've removed the earlier patch and installed these changes. Now testing
it!
> Thanks.
>
> -----Original Message-----
>  (defun query-replace-read-args (prompt regexp-flag &optional noerror)
>    (unless noerror
>      (barf-if-buffer-read-only))
> +  (save-mark-and-excursion
>    (let* ((from (query-replace-read-from prompt regexp-flag))
>  	 (to (if (consp from) (prog1 (cdr from) (setq from (car from)))
>  	       (query-replace-read-to from prompt regexp-flag))))

It seems this is what we need to push to be able to close this bug report.
Indeed, it fixes only query-replace commands.  But fixing all commands
that use the minibuffer is not as easy as adding 'save-mark-and-excursion'
like in the patch above.

The problem is that 'read-from-minibuffer' is implemented in C.
If it was implemented in Lisp, it would be easy to add just
'save-mark-and-excursion', but in C this is impossible.






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

* bug#45617: <query-replace> loses the edit region. Works in 23.3, broke in 26.3
  2021-01-13 18:53                 ` Juri Linkov
  2021-01-15  1:22                   ` Bob Floyd
@ 2021-01-15 18:09                   ` Bob Floyd
  1 sibling, 0 replies; 19+ messages in thread
From: Bob Floyd @ 2021-01-15 18:09 UTC (permalink / raw)
  To: 'Juri Linkov'; +Cc: 45617

PS: The 1st patch has the same wrong behavior also!

-----Original Message-----
From: Juri Linkov [mailto:juri@linkov.net] 
Sent: Wednesday, January 13, 2021 10:54 AM
To: Bob Floyd
Cc: 'Eli Zaretskii'; 45617@debbugs.gnu.org
Subject: Re: bug#45617: <query-replace> loses the edit region. Works in
23.3, broke in 26.3

> Perhaps another way to think about this is that emacs is "mode-less".
>
> I can begin a command, recurse into another, pop out and resume the first
> command.
>
> Without the patch, or minibuffer fix, <query-replace> breaks that design.

So here is a simpler patch that fixes all query-replace commands:

diff --git a/lisp/replace.el b/lisp/replace.el
index d41dc98a0d..16b80a8fd1 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -327,6 +336,7 @@ query-replace-read-to
 (defun query-replace-read-args (prompt regexp-flag &optional noerror)
   (unless noerror
     (barf-if-buffer-read-only))
+  (save-mark-and-excursion
   (let* ((from (query-replace-read-from prompt regexp-flag))
 	 (to (if (consp from) (prog1 (cdr from) (setq from (car from)))
 	       (query-replace-read-to from prompt regexp-flag))))
@@ -334,7 +344,7 @@ query-replace-read-args
 	  (or (and current-prefix-arg (not (eq current-prefix-arg '-)))
               (and (plist-member (text-properties-at 0 from)
'isearch-regexp-function)
                    (get-text-property 0 'isearch-regexp-function from)))
-	  (and current-prefix-arg (eq current-prefix-arg '-)))))
+	  (and current-prefix-arg (eq current-prefix-arg '-))))))
 
 (defun query-replace (from-string to-string &optional delimited start end
backward region-noncontiguous-p)
   "Replace some occurrences of FROM-STRING with TO-STRING.






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

end of thread, other threads:[~2021-01-15 18:09 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-03  1:05 bug#45617: <query-replace> loses the edit region. Works in 23.3, broke in 26.3 Bob Floyd
2021-01-03 14:47 ` Eli Zaretskii
2021-01-03 19:18   ` Bob Floyd
2021-01-04 17:26     ` Eli Zaretskii
2021-01-04 17:37   ` Juri Linkov
2021-01-04 22:40     ` Bob Floyd
2021-01-05 18:27       ` Juri Linkov
2021-01-05 19:45         ` Bob Floyd
2021-01-06 17:44           ` Juri Linkov
2021-01-06 18:19             ` Eli Zaretskii
2021-01-06 22:10               ` Bob Floyd
2021-01-13 18:53                 ` Juri Linkov
2021-01-15  1:22                   ` Bob Floyd
2021-01-15  8:54                     ` Juri Linkov
2021-01-15 11:23                       ` Eli Zaretskii
2021-01-15 18:05                       ` Bob Floyd
2021-01-15 18:09                   ` Bob Floyd
2021-01-07 18:46               ` Juri Linkov
2021-01-07 19:36                 ` Eli Zaretskii

unofficial mirror of bug-gnu-emacs@gnu.org 

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://yhetil.org/emacs-bugs/0 emacs-bugs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 emacs-bugs emacs-bugs/ https://yhetil.org/emacs-bugs \
		bug-gnu-emacs@gnu.org
	public-inbox-index emacs-bugs

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.yhetil.org/yhetil.emacs.bugs
	nntp://news.gmane.io/gmane.emacs.bugs


AGPL code for this site: git clone http://ou63pmih66umazou.onion/public-inbox.git