GNU bug report logs -
#78221
31.0.50; Improving *-change-functions notifications
Previous Next
Full log
View this message in rfc822 format
>> Alan Mackenzie made significant improvements in the past to the C code
>> that runs these hooks, so that we now rarely break our promises in terms
>> of how/when those notifications are sent, but there are still
>> cases lurking.
> By "those promises", I think you mean being that we invoke
> before-change-functions and after-change-functions strictly in matched
> pairs.
No, I mean the looser promise that AFTERs are allowed to touch only text
within the region described by the last BEFORE.
> On 2018-01-06, we formally agreed that these hooks will be invoked as
> follows:
> o - Every buffer changing primitive starts off with exactly one call
> to b-c-f,
> o - after which there will be zero, one, or several calls to a-c-f.
> o - The b-c-f's region will enclose those of all the a-c-f regions,
> but need not be the minimal such region.
Exactly. And it's now documented in the manual.
> I actually think a good strategy would be to invoke these hooks
> strictly in matching pairs in all cases, but I'm sure I said that back
> in 2018 too.
🙂
>> The main problem is that it's very hard to find the places where this
>> occurs and it's not always obvious that the corresponding fix is
>> harmless. A fix is likely to involve changes visible to the Elisp side,
>> such as new hooks, so it seemed out of scope for bug#78042.
>
> I don't think it's all that hard. In 2020, I made a list of which
> non-static functions in insdel.c call before/after-change-functions.
> The list, which is likely still valid, or very nearly so, looked like
> this:
>
> insert B/A
> insert_and_inherit B/A
> insert_char B/A
> insert_string B/A
> insert_before_markers B/A
>
> insert_before_markers_and_inherit B/A
> insert_1_both ?/-
> insert_from_string B/A
> insert_from_string_before_markers B/A
> insert_from_gap_1 -/-
>
> insert_from_gap -/-
> insert_from_buffer B/A
> del_range B/A
> del_range_1 ?/A
> del_range_byte B/A
>
> del_range_both ?/A
> del_range_2 -/-
> replace_range ?/A
> replace_range_2 -/-
> modify_text B/-
Thanks, that's very helpful.
[ It lacks the ones coming from text-property modifications. ]
The ones above that are neither B/A nor -/- are the reason why making
changes in this area is ... delicate.
> But if an alternative is to allow nested calls to the hooks (something
> we don't allow at the moment, even though the rule isn't explicitly
> formulated anywhere),
The rule is implicitly formulated where we document that AFTERs can't
touch text outside of the last BEFORE.
> I think it was a mistake in the beginning to have text property
> modifications trigger before/after-c-f.
Agreed.
> But seeing it's been that way for so long, I don't think it would be
> a good idea to change it now.
FWIW, I'm now running my Emacs with such a change to see if the ELisp
code I use cares. But yeah, it seems too risky to do it without
providing some backward compatibility.
> before/after-change-functions functions SHOULDN'T themselves make
> changes to the buffer text (apart from text properties).
+1
> I don't think it would be all that hard to instrument insdel.c to
> signal an error when such changes do get made, just tedious.
> Running such an instrumented Emacs on the test suite would catch
> a very great number of violations.
It might be worthwhile adding such runtime tests and emit warnings when
before/after-change-functions make changes the buffer text, tho it's
orthogonal to what we're discussing here (where the nesting is not due
to ill-behaved before/after-change-functions but come straight from the
C code).
> As I proposed in my reply to Stefan M., I don't think finding most/all of
> these situations need be too difficult. insdel.c could be temporarily
> instrumented to signal an error on detecting an illegitimate buffer
> change. The difficulty here might be false positives, when a program
> intends to make buffer changes from inside a change hook function. I
> don't think these are common, though.
I'm running my Emacs sessions with a function added globally to
before/after-change-functions, which checks that we stick to our
promises (basically the `sanity-check-change-functions-*` code in
`test/src/editfns-tests.el`).
But that catches only those occurrences that are triggered by my
usage patterns.
Stefan
This bug report was last modified 32 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.