[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: /* FIXME: Call signal_after_change! */ in callproc.c. Well, why no
From: |
Alan Mackenzie |
Subject: |
Re: /* FIXME: Call signal_after_change! */ in callproc.c. Well, why not? |
Date: |
Sat, 4 Jan 2020 22:47:30 +0000 |
User-agent: |
Mutt/1.10.1 (2018-07-13) |
Hello, Eli.
On Fri, Jan 03, 2020 at 10:45:53 +0200, Eli Zaretskii wrote:
> > Date: Sun, 29 Dec 2019 13:34:36 +0000
> > Cc: address@hidden
> > From: Alan Mackenzie <address@hidden>
> > Well, the more we can issue balanced before- and after- calls, the
> > better.
> I wasn't suggesting to produce unbalanced calls, I was suggesting that
> calling the hooks in a balanced way twice for the same position cannot
> be a serious problem.
It might, for example, cause some hook to invalidate some cache
unnecessarily. But even if it caused no specific problem, it would cost
a needless runtime penalty.
[ .... ]
> > We have a real existing bug here, and any fix to it runs the risk of
> > further bugs.
> Sure, but the more complex the fix, the higher the risk. It's
> uneconomical to make fixes that are more complex than strictly
> necessary, I'm sure you agree.
I think I disagree with you generally on this point - I think it is
uneconomical to install a simple workaround when a complete fix is only a
little more difficult. This change of mine is NOT complicated.
[ .... ]
> It falls short of what I'd like to see, because it doesn't cover the
> situation where this test:
> if (display_on_the_fly
> && CODING_REQUIRE_DETECTION (&saved_coding)
> && ! CODING_REQUIRE_DETECTION (&process_coding))
> causes us to switch from using the 'else' branch to using the 'else if'
> branch in the following snippet:
> if (!nread)
> ;
> else if (NILP (BVAR (current_buffer, enable_multibyte_characters))
> && ! CODING_MAY_REQUIRE_DECODING (&process_coding))
> insert_1_both (buf, nread, nread, 0, 1, 0);
> else
> { /* We have to decode the input. */
> IOW, the commentary you wrote doesn't tell the reader what
> insert_1_both, decode_coding_c_string, and del_range_2 do (or don't
> do) with regards to the modification hooks, and without that the
> comment is incomplete, and doesn't explain the logic of what the code
> does.
OK. I have to say here, I really don't believe such an extensive
commentary is needed here. The code is there, and anybody generally
familiar with our C code would understand it without a great deal of
difficulty, even the mechanism which prevents a spurious second call to
prepare_to_modify_buffer. Surely?
Anyhow I've spent quite a bit of time trying to get the level of this
comment right, and my current version of it reads as follows:
/* CHANGE FUNCTIONS
For each iteration of the enclosing while (1) loop which
yields data (i.e. nread > 0), before- and
after-change-functions are each invoked exactly once.
This is done directly from the current function only, by
calling prepare_to_modify_buffer and signal_after_change.
It is never done by directing another function such as
insert_1_both to call them. The call to
prepare_to_modify_buffer follows this comment, and
there is one call to signal_after_change in each of the
branches of the next `else if'.
Exceptionally, the insertion into the buffer is aborted
at the call to del_range_2 ~45 lines further down, this
function removing the newly inserted data. At this stage
prepare_to_modify_buffer has been called, but
signal_after_change hasn't. A continue statement
restarts the enclosing while(1) loop. A second,
unwanted, call to `prepare_to_modify_buffer' is inhibited
by the test prepared_pos < PT. The data are inserted
again, and this time signal_after_change gets called,
balancing the previous call to prepare_to_modify_buffer.
*/
> Thanks (and apologies for a delay in responding).
That's OK, I haven't exactly been prompt myself in posting to this
thread.
--
Alan Mackenzie (Nuremberg, Germany).
- Re: /* FIXME: Call signal_after_change! */ in callproc.c. Well, why not?, Eli Zaretskii, 2020/01/03
- Re: /* FIXME: Call signal_after_change! */ in callproc.c. Well, why not?,
Alan Mackenzie <=
- Re: /* FIXME: Call signal_after_change! */ in callproc.c. Well, why not?, Eli Zaretskii, 2020/01/05
- Re: /* FIXME: Call signal_after_change! */ in callproc.c. Well, why not?, Alan Mackenzie, 2020/01/05
- Re: /* FIXME: Call signal_after_change! */ in callproc.c. Well, why not?, Alan Mackenzie, 2020/01/21
- Re: /* FIXME: Call signal_after_change! */ in callproc.c. Well, why not?, Eli Zaretskii, 2020/01/21
- Re: /* FIXME: Call signal_after_change! */ in callproc.c. Well, why not?, Alan Mackenzie, 2020/01/22