unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#42424: 27.0.90; replace-match: point is NOT left at the end of replacement
@ 2020-07-19  5:50 Ren Victor
  2020-10-17  9:49 ` Lars Ingebrigtsen
  2024-04-09 15:14 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 2 replies; 11+ messages in thread
From: Ren Victor @ 2020-07-19  5:50 UTC (permalink / raw)
  To: 42424

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

I attach an ert case to show the recipe.

This issue happens when modification hooks modify the text before the
end of replacement text.

`replace-match' calls `replace_range' to do the actual replacement,
which can trigger modification hooks.  Before calling `replace_range',
the position of the end of replacement is saved. After `replace_range',
the point is moved to the saved position.

But the end of replacement might be changed inside of
`replace_range'.  So the final movement of point may end up to a wrong
place.

Other types of modification (insert or delete) do not have this issue.
`point' is adjusted before running modification hooks.

In `replace_range', the point is also relocated.  I am not sure why it has
to be moved again just before returning from `replace-match'.

This is not a new issue.  It is in Emacs24 at least.


In GNU Emacs 27.0.90 (build 1, x86_64-pc-cygwin)
 of 2020-03-04 built on moufang2
Repository revision: afff43a72e96fcccabe77ff63226cddd540e068d
Repository branch: master
Windowing system distributor 'Microsoft Corp.', version 10.0.19041
Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Quit
ert
next-line: End of buffer [3 times]
test-replace-match
Ran 1 tests, 0 results were as expected, 1 unexpected
next-line: End of buffer
previous-line: Beginning of buffer [13 times]
previous-line: Beginning of buffer
Configured using:
 'configure
 --srcdir=/home/kbrown/src/cygpackages/emacs/emacs-27.0.90-1.x86_64/src/emacs-27.0.90
 --prefix=/usr --exec-prefix=/usr --localstatedir=/var --sysconfdir=/etc
 --docdir=/usr/share/doc/emacs --htmldir=/usr/share/doc/emacs/html -C
 --with-w32 'CFLAGS=-ggdb -O2 -pipe -Wall -Werror=format-security
 -Wp,-D_FORTIFY_SOURCE=2 -fstack-protector-strong
 --param=ssp-buffer-size=4
 -fdebug-prefix-map=/home/kbrown/src/cygpackages/emacs/emacs-27.0.90-1.x86_64/build=/usr/src/debug/emacs-27.0.90-1
 -fdebug-prefix-map=/home/kbrown/src/cygpackages/emacs/emacs-27.0.90-1.x86_64/src/emacs-27.0.90=/usr/src/debug/emacs-27.0.90-1'
 CPPFLAGS= LDFLAGS='

Configured features:
XPM JPEG TIFF GIF PNG SOUND DBUS GLIB NOTIFY GFILENOTIFY ACL GNUTLS
LIBXML2 HARFBUZZ ZLIB TOOLKIT_SCROLL_BARS XIM MODULES THREADS JSON
PDUMPER LCMS2 GMP

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Emacs-Lisp

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
format-spec rfc822 mml mml-sec password-cache epa derived epg epg-config
gnus-util rmail rmail-loaddefs text-property-search time-date subr-x
mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils
mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr
mail-utils cl-seq cl-extra seq byte-opt bytecomp byte-compile cconv
cl-macs gv ert pp ewoc debug backtrace help-mode find-func vc-git
diff-mode easymenu easy-mmode cl-loaddefs cl-lib tooltip eldoc electric
uniquify ediff-hook vc-hooks lisp-float-type mwheel disp-table
term/w32-win w32-win w32-vars term/common-win tool-bar dnd fontset image
regexp-opt fringe tabulated-list replace newcomment text-mode elisp-mode
lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow isearch
timer select scroll-bar mouse jit-lock font-lock syntax facemenu
font-core term/tty-colors frame minibuffer cl-generic cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european
ethiopic indian cyrillic chinese composite charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote threads dbusbind
gfilenotify w32 lcms2 multi-tty make-network-process emacs)

Memory information:
((conses 16 62659 8699)
 (symbols 48 7750 1)
 (strings 32 21627 1406)
 (string-bytes 1 668356)
 (vectors 16 12971)
 (vector-slots 8 156156 9450)
 (floats 8 40 94)
 (intervals 56 307 1)
 (buffers 1000 14))

[-- Attachment #2: test_replace_match.el --]
[-- Type: application/octet-stream, Size: 525 bytes --]

(require 'ert)

(ert-deftest test-replace-match ()
  "Test for bug."
  (let ((check-point nil)
	(ov-set nil))
    (with-temp-buffer
      (insert "a abc")
      (setq ov-set (make-overlay 3 5))
      (overlay-put
	   ov-set 'modification-hooks
	   (list (lambda (o after &rest _args)
			   (when after
				 (let ((inhibit-modification-hooks t))
				   (save-excursion
					 (goto-char 2)
					 (insert "bcd")))))))
      (goto-char 3)
	  (if (search-forward "bc")
		  (replace-match "bcd"))
      (should (eq (point) 10)))))

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

* bug#42424: 27.0.90; replace-match: point is NOT left at the end of replacement
  2020-07-19  5:50 bug#42424: 27.0.90; replace-match: point is NOT left at the end of replacement Ren Victor
@ 2020-10-17  9:49 ` Lars Ingebrigtsen
  2020-10-17 17:25   ` Eli Zaretskii
  2024-04-09 15:14 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 1 reply; 11+ messages in thread
From: Lars Ingebrigtsen @ 2020-10-17  9:49 UTC (permalink / raw)
  To: Ren Victor; +Cc: 42424

Ren Victor <victorhge@gmail.com> writes:

> In `replace_range', the point is also relocated.  I am not sure why it has
> to be moved again just before returning from `replace-match'.

I can reproduce this error on Emacs 28.

The problem seems to be this in Freplace_match?  

  /* Put point back where it was in the text, if possible.  */
  TEMP_SET_PT (clip_to_bounds (BEGV, opoint + (opoint <= 0 ? ZV : 0), ZV));
  /* Now move point "officially" to the start of the inserted replacement.  */
  move_if_not_intangible (newpoint);

Uhm...  is that comment wrong?  Aren't we moving point to the end of the
inserted replacement?

Anyway, removing that move makes the suggested test not fail, but it
leads to a bunch of other tests failing, so it's doing something right,
at least.

However, just before that, there's this:

  /* The replace_range etc. functions can trigger modification hooks
     (see signal_before_change and signal_after_change).  Try to error
     out if these hooks clobber the match data since clobbering can
     result in confusing bugs.  Although this sanity check does not
     catch all possible clobberings, it should catch many of them.  */
  if (! (search_regs.num_regs == num_regs
	 && search_regs.start[sub] == newstart
	 && search_regs.end[sub] == newpoint))
    error ("Match data clobbered by buffer modification hooks");

So replace_match is already signalling some errors on buffer
modification hooks, but not here.  So is this a bug or just something
that should be documented better?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





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

* bug#42424: 27.0.90; replace-match: point is NOT left at the end of replacement
  2020-10-17  9:49 ` Lars Ingebrigtsen
@ 2020-10-17 17:25   ` Eli Zaretskii
  2020-10-18  8:24     ` Lars Ingebrigtsen
  0 siblings, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2020-10-17 17:25 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: victorhge, 42424

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Sat, 17 Oct 2020 11:49:42 +0200
> Cc: 42424@debbugs.gnu.org
> 
>   /* Put point back where it was in the text, if possible.  */
>   TEMP_SET_PT (clip_to_bounds (BEGV, opoint + (opoint <= 0 ? ZV : 0), ZV));
>   /* Now move point "officially" to the start of the inserted replacement.  */
>   move_if_not_intangible (newpoint);
> 
> Uhm...  is that comment wrong?  Aren't we moving point to the end of the
> inserted replacement?

Yes, it should say "end", not "start".

>   /* The replace_range etc. functions can trigger modification hooks
>      (see signal_before_change and signal_after_change).  Try to error
>      out if these hooks clobber the match data since clobbering can
>      result in confusing bugs.  Although this sanity check does not
>      catch all possible clobberings, it should catch many of them.  */
>   if (! (search_regs.num_regs == num_regs
> 	 && search_regs.start[sub] == newstart
> 	 && search_regs.end[sub] == newpoint))
>     error ("Match data clobbered by buffer modification hooks");
> 
> So replace_match is already signalling some errors on buffer
> modification hooks, but not here.  So is this a bug or just something
> that should be documented better?

Personally, I wonder what was expected here.  If the modification
hooks modify the replaced text behind our back, how can the Lisp
program which does that expect to have point where it belongs?  Am I
missing something?





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

* bug#42424: 27.0.90; replace-match: point is NOT left at the end of replacement
  2020-10-17 17:25   ` Eli Zaretskii
@ 2020-10-18  8:24     ` Lars Ingebrigtsen
  2021-07-31 14:03       ` Lars Ingebrigtsen
  0 siblings, 1 reply; 11+ messages in thread
From: Lars Ingebrigtsen @ 2020-10-18  8:24 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: victorhge, 42424

Eli Zaretskii <eliz@gnu.org> writes:

>> Uhm...  is that comment wrong?  Aren't we moving point to the end of the
>> inserted replacement?
>
> Yes, it should say "end", not "start".

Fixed now.

> Personally, I wonder what was expected here.  If the modification
> hooks modify the replaced text behind our back, how can the Lisp
> program which does that expect to have point where it belongs?  Am I
> missing something?

No, the semantics are pretty unclear, and it's not obvious whether we
can guarantee anything here.  But the bug reporter notes:

> Other types of modification (insert or delete) do not have this issue.
> `point' is adjusted before running modification hooks.

So I think the suggestion is to move point to the end of the replacement
before running the modification hooks, i.e., move the point logic to
replace_range.  But I'm not actually sure where the modification hooks
are being run from...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





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

* bug#42424: 27.0.90; replace-match: point is NOT left at the end of replacement
  2020-10-18  8:24     ` Lars Ingebrigtsen
@ 2021-07-31 14:03       ` Lars Ingebrigtsen
  2021-07-31 14:20         ` Eli Zaretskii
  0 siblings, 1 reply; 11+ messages in thread
From: Lars Ingebrigtsen @ 2021-07-31 14:03 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 42424, victorhge

Lars Ingebrigtsen <larsi@gnus.org> writes:

>> Other types of modification (insert or delete) do not have this issue.
>> `point' is adjusted before running modification hooks.
>
> So I think the suggestion is to move point to the end of the replacement
> before running the modification hooks, i.e., move the point logic to
> replace_range.

No, that was wrong -- the suggestion was to not move point to the end of
the replacement text (numerically speaking).  But we can't do that --
the `replace-match' interface guarantees that it will do that, and
changing this will break stuff.

The modification hooks may do all kinda of oddball stuff that changes
the buffer before moving to that point, so to make that work more
reliably, we'd have to make a marker, I think?  But `replace-match' is
used so heavily that I'm not sure the performance impact won't be
noticeable.

One possible idea is to postpone the modification hooks until the end of
replace_match (instead of doing it in replace_range -- that should have
no performance impact, and not change behaviour in most cases.  But I'm
having problems making that work.

The hook is called from update_compositions, isn't it?  Are there any
caveats to using that?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





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

* bug#42424: 27.0.90; replace-match: point is NOT left at the end of replacement
  2021-07-31 14:03       ` Lars Ingebrigtsen
@ 2021-07-31 14:20         ` Eli Zaretskii
  2021-07-31 14:28           ` Lars Ingebrigtsen
  0 siblings, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2021-07-31 14:20 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 42424, victorhge

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: 42424@debbugs.gnu.org,  victorhge@gmail.com
> Date: Sat, 31 Jul 2021 16:03:44 +0200
> 
> The hook is called from update_compositions, isn't it?  Are there any
> caveats to using that?

No, the hooks are called from signal_after_change, AFAICT.

And I'm probably missing something, because I don't understand how
calling the hooks from replace-match would help: replace_range is
called just once from replace-match, and the hooks are invoked at its
very end.  What am I missing?





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

* bug#42424: 27.0.90; replace-match: point is NOT left at the end of replacement
  2021-07-31 14:20         ` Eli Zaretskii
@ 2021-07-31 14:28           ` Lars Ingebrigtsen
  2021-07-31 14:49             ` Lars Ingebrigtsen
  0 siblings, 1 reply; 11+ messages in thread
From: Lars Ingebrigtsen @ 2021-07-31 14:28 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 42424, victorhge

Eli Zaretskii <eliz@gnu.org> writes:

> No, the hooks are called from signal_after_change, AFAICT.

Hm, not able to get that working, either...

> And I'm probably missing something, because I don't understand how
> calling the hooks from replace-match would help: replace_range is
> called just once from replace-match, and the hooks are invoked at its
> very end.  What am I missing?

Because replace-match does this:

    replace_range (sub_start, sub_end, newtext, 1, 0, 1, true, true);

[...]

  /* Now move point "officially" to the end of the inserted replacement.  */
  move_if_not_intangible (newpoint);

And that leaves point somewhere odd if the hook has inserted more text
at the start of the buffer.

My idea was to try to see whether moving the hook stuff later would fix
the issue (and not regress anything).  Basically:

diff --git a/src/search.c b/src/search.c
index df384e1dcf..2c0d58c523 100644
--- a/src/search.c
+++ b/src/search.c
@@ -2725,15 +2726,21 @@ DEFUN ("replace-match", Freplace_match, Sreplace_match, 1, 5, 0,
   newpoint = sub_start + SCHARS (newtext);
 
   /* Replace the old text with the new in the cleanest possible way.  */
-  replace_range (sub_start, sub_end, newtext, 1, 0, 1, true);
-
-  if (case_action == all_caps)
-    Fupcase_region (make_fixnum (search_regs.start[sub]),
-		    make_fixnum (newpoint),
-		    Qnil);
-  else if (case_action == cap_initial)
-    Fupcase_initials_region (make_fixnum (search_regs.start[sub]),
-			     make_fixnum (newpoint), Qnil);
+  {
+    ptrdiff_t count = SPECPDL_INDEX ();
+    specbind (Qinhibit_modification_hooks, Qt);
+
+    replace_range (sub_start, sub_end, newtext, 1, 0, 1, true);
+
+    if (case_action == all_caps)
+      Fupcase_region (make_fixnum (search_regs.start[sub]),
+		      make_fixnum (newpoint),
+		      Qnil);
+    else if (case_action == cap_initial)
+      Fupcase_initials_region (make_fixnum (search_regs.start[sub]),
+			       make_fixnum (newpoint), Qnil);
+    unbind_to (count, Qnil);
+  }
 
   /* The replace_range etc. functions can trigger modification hooks
      (see signal_before_change and signal_after_change).  Try to error
@@ -2750,6 +2757,9 @@ DEFUN ("replace-match", Freplace_match, Sreplace_match, 1, 5, 0,
   /* Now move point "officially" to the end of the inserted replacement.  */
   move_if_not_intangible (newpoint);
 
+  signal_after_change (sub_start, sub_end - sub_start, SCHARS (newtext));
+  update_compositions (sub_start, newpoint, CHECK_BORDER);
+  
   return Qnil;
 }


But that does not seem to call the modification hook at all in the test
case.  Am I doing something obviously wrong here?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





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

* bug#42424: 27.0.90; replace-match: point is NOT left at the end of replacement
  2021-07-31 14:28           ` Lars Ingebrigtsen
@ 2021-07-31 14:49             ` Lars Ingebrigtsen
  2021-07-31 15:10               ` Eli Zaretskii
  0 siblings, 1 reply; 11+ messages in thread
From: Lars Ingebrigtsen @ 2021-07-31 14:49 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 42424, victorhge

Lars Ingebrigtsen <larsi@gnus.org> writes:

> But that does not seem to call the modification hook at all in the test
> case.  Am I doing something obviously wrong here?

Er.  I may just have been testing the wrong thing or something.  I redid
the patch by adding a new parameter to replace_range to inhibit it
calling the hook, and now the test case works fine, and it doesn't break
any tests.  (And it seems to work more logically all over.)

Any comments?  I've tried to imagine how it might regress something for
somebody, but I don't think it should -- we're calling the modification
hook at the same point as before (I think), for instance.

diff --git a/src/cmds.c b/src/cmds.c
index c8a96d918c..00fde0ef79 100644
--- a/src/cmds.c
+++ b/src/cmds.c
@@ -455,7 +455,7 @@ internal_self_insert (int c, EMACS_INT n)
       ptrdiff_t to;
       if (INT_ADD_WRAPV (PT, chars_to_delete, &to))
 	to = PTRDIFF_MAX;
-      replace_range (PT, to, string, 1, 1, 1, 0);
+      replace_range (PT, to, string, 1, 1, 1, 0, false);
       Fforward_char (make_fixnum (n));
     }
   else if (n > 1)
diff --git a/src/editfns.c b/src/editfns.c
index 8ab17ebc9f..c8219decb0 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -2371,7 +2371,7 @@ #define COMBINING_BOTH (COMBINING_BEFORE | COMBINING_AFTER)
 	      /* replace_range is less efficient, because it moves the gap,
 		 but it handles combining correctly.  */
 	      replace_range (pos, pos + 1, string,
-			     false, false, true, false);
+			     false, false, true, false, false);
 	      pos_byte_next = CHAR_TO_BYTE (pos);
 	      if (pos_byte_next > pos_byte)
 		/* Before combining happened.  We should not increment
@@ -2578,7 +2578,7 @@ DEFUN ("translate-region-internal", Ftranslate_region_internal,
 		     but it should handle multibyte characters correctly.  */
 		  string = make_multibyte_string ((char *) str, 1, str_len);
 		  replace_range (pos, pos + 1, string,
-				 true, false, true, false);
+				 true, false, true, false, false);
 		  len = str_len;
 		}
 	      else
@@ -2613,7 +2613,8 @@ DEFUN ("translate-region-internal", Ftranslate_region_internal,
 		= (VECTORP (val)
 		   ? Fconcat (1, &val)
 		   : Fmake_string (make_fixnum (1), val, Qnil));
-	      replace_range (pos, pos + len, string, true, false, true, false);
+	      replace_range (pos, pos + len, string, true, false, true, false,
+			     false);
 	      pos_byte += SBYTES (string);
 	      pos += SCHARS (string);
 	      characters_changed += SCHARS (string);
diff --git a/src/insdel.c b/src/insdel.c
index e66120eb08..92ee2c42ea 100644
--- a/src/insdel.c
+++ b/src/insdel.c
@@ -1392,7 +1392,7 @@ adjust_after_insert (ptrdiff_t from, ptrdiff_t from_byte,
 void
 replace_range (ptrdiff_t from, ptrdiff_t to, Lisp_Object new,
                bool prepare, bool inherit, bool markers,
-               bool adjust_match_data)
+               bool adjust_match_data, bool inhibit_update_compositions)
 {
   ptrdiff_t inschars = SCHARS (new);
   ptrdiff_t insbytes = SBYTES (new);
@@ -1552,8 +1552,11 @@ replace_range (ptrdiff_t from, ptrdiff_t to, Lisp_Object new,
   if (adjust_match_data)
     update_search_regs (from, to, from + SCHARS (new));
 
-  signal_after_change (from, nchars_del, GPT - from);
-  update_compositions (from, GPT, CHECK_BORDER);
+  if (!inhibit_update_compositions)
+    {
+      signal_after_change (from, nchars_del, GPT - from);
+      update_compositions (from, GPT, CHECK_BORDER);
+    }
 }
 \f
 /* Replace the text from character positions FROM to TO with
diff --git a/src/lisp.h b/src/lisp.h
index 15a42a4456..1206a0d1f6 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -3717,7 +3717,8 @@ verify (FLT_RADIX == 2 || FLT_RADIX == 16);
 				       ptrdiff_t, ptrdiff_t);
 extern void adjust_markers_bytepos (ptrdiff_t, ptrdiff_t,
 				    ptrdiff_t, ptrdiff_t, int);
-extern void replace_range (ptrdiff_t, ptrdiff_t, Lisp_Object, bool, bool, bool, bool);
+extern void replace_range (ptrdiff_t, ptrdiff_t, Lisp_Object, bool, bool,
+			   bool, bool, bool);
 extern void replace_range_2 (ptrdiff_t, ptrdiff_t, ptrdiff_t, ptrdiff_t,
 			     const char *, ptrdiff_t, ptrdiff_t, bool);
 extern void syms_of_insdel (void);
diff --git a/src/search.c b/src/search.c
index df384e1dcf..a295ca12bd 100644
--- a/src/search.c
+++ b/src/search.c
@@ -30,6 +30,7 @@ Copyright (C) 1985-1987, 1993-1994, 1997-1999, 2001-2021 Free Software
 #include "blockinput.h"
 #include "intervals.h"
 #include "pdumper.h"
+#include "composite.h"
 
 #include "regex-emacs.h"
 
@@ -2725,8 +2726,8 @@ DEFUN ("replace-match", Freplace_match, Sreplace_match, 1, 5, 0,
   newpoint = sub_start + SCHARS (newtext);
 
   /* Replace the old text with the new in the cleanest possible way.  */
-  replace_range (sub_start, sub_end, newtext, 1, 0, 1, true);
-
+  replace_range (sub_start, sub_end, newtext, 1, 0, 1, true, true);
+  
   if (case_action == all_caps)
     Fupcase_region (make_fixnum (search_regs.start[sub]),
 		    make_fixnum (newpoint),
@@ -2750,6 +2751,9 @@ DEFUN ("replace-match", Freplace_match, Sreplace_match, 1, 5, 0,
   /* Now move point "officially" to the end of the inserted replacement.  */
   move_if_not_intangible (newpoint);
 
+  signal_after_change (sub_start, sub_end - sub_start, SCHARS (newtext));
+  update_compositions (sub_start, newpoint, CHECK_BORDER);
+
   return Qnil;
 }
 \f
diff --git a/test/src/search-tests.el b/test/src/search-tests.el
new file mode 100644
index 0000000000..b7b4ab9a8f
--- /dev/null
+++ b/test/src/search-tests.el
@@ -0,0 +1,42 @@
+;;; search-tests.el --- tests for search.c functions -*- lexical-binding: t -*-
+
+;; Copyright (C) 2015-2016, 2018-2021 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Code:
+
+(require 'ert)
+
+(ert-deftest test-replace-match-modification-hooks ()
+  (let ((ov-set nil))
+    (with-temp-buffer
+      (insert "1 abc")
+      (setq ov-set (make-overlay 3 5))
+      (overlay-put
+       ov-set 'modification-hooks
+       (list (lambda (o after &rest _args)
+	       (when after
+		 (let ((inhibit-modification-hooks t))
+		   (save-excursion
+		     (goto-char 2)
+		     (insert "234")))))))
+      (goto-char 3)
+      (if (search-forward "bc")
+	  (replace-match "bcd"))
+      (should (= (point) 10)))))
+
+;;; search-tests.el ends here


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





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

* bug#42424: 27.0.90; replace-match: point is NOT left at the end of replacement
  2021-07-31 14:49             ` Lars Ingebrigtsen
@ 2021-07-31 15:10               ` Eli Zaretskii
  2021-07-31 15:45                 ` Lars Ingebrigtsen
  0 siblings, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2021-07-31 15:10 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 42424, victorhge

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: 42424@debbugs.gnu.org,  victorhge@gmail.com
> Date: Sat, 31 Jul 2021 16:49:52 +0200
> 
>  void
>  replace_range (ptrdiff_t from, ptrdiff_t to, Lisp_Object new,
>                 bool prepare, bool inherit, bool markers,
> -               bool adjust_match_data)
> +               bool adjust_match_data, bool inhibit_update_compositions)

The name is misleading: the 2 functions whose call you want to bypass
are not just for updating compositions.





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

* bug#42424: 27.0.90; replace-match: point is NOT left at the end of replacement
  2021-07-31 15:10               ` Eli Zaretskii
@ 2021-07-31 15:45                 ` Lars Ingebrigtsen
  0 siblings, 0 replies; 11+ messages in thread
From: Lars Ingebrigtsen @ 2021-07-31 15:45 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 42424, victorhge

Eli Zaretskii <eliz@gnu.org> writes:

>>  replace_range (ptrdiff_t from, ptrdiff_t to, Lisp_Object new,
>>                 bool prepare, bool inherit, bool markers,
>> -               bool adjust_match_data)
>> +               bool adjust_match_data, bool inhibit_update_compositions)
>
> The name is misleading: the 2 functions whose call you want to bypass
> are not just for updating compositions.

Yup; now renamed and pushed.

It's hard to say whether this might have strange unforeseen effects, but
this change makes `replace-match' do what it's documented to do, at least.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





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

* bug#42424: 27.0.90; replace-match: point is NOT left at the end of replacement
  2020-07-19  5:50 bug#42424: 27.0.90; replace-match: point is NOT left at the end of replacement Ren Victor
  2020-10-17  9:49 ` Lars Ingebrigtsen
@ 2024-04-09 15:14 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 0 replies; 11+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-04-09 15:14 UTC (permalink / raw)
  To: Ren Victor; +Cc: 42424

> This issue happens when modification hooks modify the text before the
> end of replacement text.

Modification hooks which modify the buffer text are just getting what
they deserve.

> But the end of replacement might be changed inside of `replace_range'.
> So the final movement of point may end up to a wrong place.

This is just one of the many problems.

> Other types of modification (insert or delete) do not have this issue.
> `point' is adjusted before running modification hooks.

Yes, occasionally doing bad things won't bite you in the rear.
That doesn't justify doing those things.

If you want to modify the buffer in response to other buffer
modifications, then record this fact in the modification hook and then
act on it later, e.g. in a timer.
Anything else *will* cause problems sooner or later.


        Stefan






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

end of thread, other threads:[~2024-04-09 15:14 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-19  5:50 bug#42424: 27.0.90; replace-match: point is NOT left at the end of replacement Ren Victor
2020-10-17  9:49 ` Lars Ingebrigtsen
2020-10-17 17:25   ` Eli Zaretskii
2020-10-18  8:24     ` Lars Ingebrigtsen
2021-07-31 14:03       ` Lars Ingebrigtsen
2021-07-31 14:20         ` Eli Zaretskii
2021-07-31 14:28           ` Lars Ingebrigtsen
2021-07-31 14:49             ` Lars Ingebrigtsen
2021-07-31 15:10               ` Eli Zaretskii
2021-07-31 15:45                 ` Lars Ingebrigtsen
2024-04-09 15:14 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors

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).