Subject: bug#42424: 27.0.90; replace-match: point is NOT left at the end of replacement

From: Lars Ingebrigtsen <[email-masked]>
Date: Sat, 31 Jul 2021 16:28:54 +0200

Eli Zaretskii <[email-masked]> 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