From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefan Monnier Subject: =?UTF-8?B?YnVnIzIzOTE3OiBQbGVhc2UgY29uc2lkZXIgbWFraW5nIEJ1ZyAj?= =?UTF-8?B?MjM5MTcgYSBibG9ja2VyIGZvciAyNS4xICh3YXMgUmU6IG9yZy1jYXB0dXJl?= =?UTF-8?B?OiBDYXB0dXJlIHRlbXBsYXRlIOKAmGfigJk6IE1hdGNoIGRhdGEgY2xvYmJl?= =?UTF-8?B?cmVkIGJ5IGJ1ZmZlciBtb2RpZmljYXRpb24gaG9va3Mp?= Date: Wed, 20 Jul 2016 21:47:13 -0400 Message-ID: References: <87vb066ejv.fsf@linaro.org> <87mvlhmv0x.fsf_-_@moondust.awandering> <837fcl5zs9.fsf@gnu.org> <87a8hgkwcb.fsf@linaro.org> <8360s42mcb.fsf@gnu.org> <87eg6rgmlg.fsf@gmail.com> <83lh0y24y6.fsf@gnu.org> <83eg6q1hbo.fsf@gnu.org> <83a8hd1vzi.fsf@gnu.org> <834m7l1u8u.fsf@gnu.org> <83shv4z7e8.fsf@gnu.org> <83inw0yw9q.fsf@gnu.org> <87bn1rdd1f.fsf@users.sourceforge.net> Mime-Version: 1.0 Content-Type: text/plain Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:59518) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bQ35v-0008Js-9s for emacs-orgmode@gnu.org; Wed, 20 Jul 2016 21:48:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bQ35t-00042j-QM for emacs-orgmode@gnu.org; Wed, 20 Jul 2016 21:48:11 -0400 Sender: "Debbugs-submit" Resent-Message-ID: In-Reply-To: <87bn1rdd1f.fsf@users.sourceforge.net> (npostavs's message of "Wed, 20 Jul 2016 20:56:28 -0400") List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Sender: "Emacs-orgmode" To: npostavs@users.sourceforge.net Cc: nljlistbox2@gmail.com, jwiegley@gmail.com, rpluim@gmail.com, 23917@debbugs.gnu.org, Eli Zaretskii , alex.bennee@linaro.org > Solution: adjust in between the before and after change functions. > Patch below. I think there shouldn't be performance problems, although > it does add yet another flag to replace_range (by the way, I noticed > that the MARKERS flags is never 0, so it's redundant). I tested with > the attached bug-23917-match-data-buffer-modhook.el. Looks pretty good, indeed. More specifically, looks like the better fix. 2 comments: - I'd take advantage of this change to replace the 0/1 booleans with false/true [but that's just my preference of bikeshed color). - maybe we can even adjust_match_data in every call to replace_range rather than just in the one from Freplace_match. Thanks, Stefan > From a8098080dff5f83f7cbcbec2bc263f9db3b45ad9 Mon Sep 17 00:00:00 2001 > From: Noam Postavsky > Date: Wed, 20 Jul 2016 20:15:14 -0400 > Subject: [PATCH v1] Adjust match data before calling after-change-funs > * src/insdel.c (replace_range): Add new parameter ADJUST_MATCH_DATA, if > true. Update all callers except Freplace_match to pass 0 for the new > parameter. > * src/search.c (update_search_regs): New function, extracted from > Freplace_match. > (Freplace_match): Remove match data adjustment code, pass 1 for > ADJUST_MATCH_DATA to replace_range instead. > --- > src/cmds.c | 2 +- > src/editfns.c | 6 +++--- > src/insdel.c | 10 ++++++++-- > src/lisp.h | 4 +++- > src/search.c | 50 +++++++++++++++++++++++++++++--------------------- > 5 files changed, 44 insertions(+), 28 deletions(-) > diff --git a/src/cmds.c b/src/cmds.c > index 1e44ddd..4003d8b 100644 > --- a/src/cmds.c > +++ b/src/cmds.c > @@ -447,7 +447,7 @@ internal_self_insert (int c, EMACS_INT n) > string = concat2 (string, tem); > } > - replace_range (PT, PT + chars_to_delete, string, 1, 1, 1); > + replace_range (PT, PT + chars_to_delete, string, 1, 1, 1, 0); > Fforward_char (make_number (n)); > } > else if (n > 1) > diff --git a/src/editfns.c b/src/editfns.c > index 412745d..32c8bec 100644 > --- a/src/editfns.c > +++ b/src/editfns.c > @@ -3192,7 +3192,7 @@ DEFUN ("subst-char-in-region", Fsubst_char_in_region, > /* replace_range is less efficient, because it moves the gap, > but it handles combining correctly. */ > replace_range (pos, pos + 1, string, > - 0, 0, 1); > + 0, 0, 1, 0); > pos_byte_next = CHAR_TO_BYTE (pos); > if (pos_byte_next > pos_byte) > /* Before combining happened. We should not increment > @@ -3405,7 +3405,7 @@ DEFUN ("translate-region-internal", Ftranslate_region_internal, > /* This is less efficient, because it moves the gap, > but it should handle multibyte characters correctly. */ > string = make_multibyte_string ((char *) str, 1, str_len); > - replace_range (pos, pos + 1, string, 1, 0, 1); > + replace_range (pos, pos + 1, string, 1, 0, 1, 0); > len = str_len; > } > else > @@ -3446,7 +3446,7 @@ DEFUN ("translate-region-internal", Ftranslate_region_internal, > { > string = Fmake_string (make_number (1), val); > } > - replace_range (pos, pos + len, string, 1, 0, 1); > + replace_range (pos, pos + len, string, 1, 0, 1, 0); > pos_byte += SBYTES (string); > pos += SCHARS (string); > cnt += SCHARS (string); > diff --git a/src/insdel.c b/src/insdel.c > index 4ad1074..fc3f19f 100644 > --- a/src/insdel.c > +++ b/src/insdel.c > @@ -1268,7 +1268,9 @@ adjust_after_insert (ptrdiff_t from, ptrdiff_t from_byte, > /* Replace the text from character positions FROM to TO with NEW, > If PREPARE, call prepare_to_modify_buffer. > If INHERIT, the newly inserted text should inherit text properties > - from the surrounding non-deleted text. */ > + from the surrounding non-deleted text. > + If ADJUST_MATCH_DATA, then adjust the match data before calling > + signal_after_change. */ > /* Note that this does not yet handle markers quite right. > Also it needs to record a single undo-entry that does a replacement > @@ -1279,7 +1281,8 @@ 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 prepare, bool inherit, bool markers, > + bool adjust_match_data) > { > ptrdiff_t inschars = SCHARS (new); > ptrdiff_t insbytes = SBYTES (new); > @@ -1426,6 +1429,9 @@ replace_range (ptrdiff_t from, ptrdiff_t to, Lisp_Object new, > MODIFF++; > CHARS_MODIFF = MODIFF; > + 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); > } > diff --git a/src/lisp.h b/src/lisp.h > index 6a98adb..25f811e 100644 > --- a/src/lisp.h > +++ b/src/lisp.h > @@ -3516,7 +3516,7 @@ extern void adjust_after_insert (ptrdiff_t, ptrdiff_t, ptrdiff_t, > ptrdiff_t, ptrdiff_t); > extern void adjust_markers_for_delete (ptrdiff_t, ptrdiff_t, > ptrdiff_t, ptrdiff_t); > -extern void replace_range (ptrdiff_t, ptrdiff_t, Lisp_Object, bool, bool, bool); > +extern void replace_range (ptrdiff_t, ptrdiff_t, Lisp_Object, 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); > @@ -3994,6 +3994,8 @@ extern Lisp_Object make_temp_name (Lisp_Object, bool); > /* Defined in search.c. */ > extern void shrink_regexp_cache (void); > extern void restore_search_regs (void); > +extern void update_search_regs (ptrdiff_t oldstart, > + ptrdiff_t oldend, ptrdiff_t newend); > extern void record_unwind_save_match_data (void); > struct re_registers; > extern struct re_pattern_buffer *compile_pattern (Lisp_Object, > diff --git a/src/search.c b/src/search.c > index 5c949ad..1b82c94 100644 > --- a/src/search.c > +++ b/src/search.c > @@ -2727,8 +2727,15 @@ DEFUN ("replace-match", Freplace_match, Sreplace_match, 1, 5, 0, > /* Replace the old text with the new in the cleanest possible way. */ > replace_range (search_regs.start[sub], search_regs.end[sub], > - newtext, 1, 0, 1); > + newtext, 1, 0, 1, 1); > newpoint = search_regs.start[sub] + SCHARS (newtext); > + /* Update saved data to match adjustment made by replace_range. */ > + { > + ptrdiff_t change = newpoint - sub_end; > + if (sub_start >= sub_end) > + sub_start += change; > + sub_end += change; > + } > if (case_action == all_caps) > Fupcase_region (make_number (search_regs.start[sub]), > @@ -2742,26 +2749,6 @@ DEFUN ("replace-match", Freplace_match, Sreplace_match, 1, 5, 0, > || search_regs.num_regs != num_regs) > error ("Match data clobbered by buffer modification hooks"); > - /* Adjust search data for this change. */ > - { > - ptrdiff_t oldend = search_regs.end[sub]; > - ptrdiff_t oldstart = search_regs.start[sub]; > - ptrdiff_t change = newpoint - search_regs.end[sub]; > - ptrdiff_t i; > - > - for (i = 0; i < search_regs.num_regs; i++) > - { > - if (search_regs.start[i] >= oldend) > - search_regs.start[i] += change; > - else if (search_regs.start[i] > oldstart) > - search_regs.start[i] = oldstart; > - if (search_regs.end[i] >= oldend) > - search_regs.end[i] += change; > - else if (search_regs.end[i] > oldstart) > - search_regs.end[i] = oldstart; > - } > - } > - > /* Put point back where it was in the text. */ > if (opoint <= 0) > TEMP_SET_PT (opoint + ZV); > @@ -3102,6 +3089,27 @@ restore_search_regs (void) > } > } > +/* Called from replace-match via replace_range. */ > +void > +update_search_regs (ptrdiff_t oldstart, ptrdiff_t oldend, ptrdiff_t newend) > +{ > + /* Adjust search data for this change. */ > + ptrdiff_t change = newend - oldend; > + ptrdiff_t i; > + > + for (i = 0; i < search_regs.num_regs; i++) > + { > + if (search_regs.start[i] >= oldend) > + search_regs.start[i] += change; > + else if (search_regs.start[i] > oldstart) > + search_regs.start[i] = oldstart; > + if (search_regs.end[i] >= oldend) > + search_regs.end[i] += change; > + else if (search_regs.end[i] > oldstart) > + search_regs.end[i] = oldstart; > + } > +} > + > static void > unwind_set_match_data (Lisp_Object list) > { > -- > 2.8.0