[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, 21 Dec 2019 21:47:52 +0000 |
User-agent: |
Mutt/1.10.1 (2018-07-13) |
Hello, Eli.
Sorry my last post was not thought through. I hope this one is better.
On Sat, Dec 21, 2019 at 20:11:01 +0200, Eli Zaretskii wrote:
> > Date: Sat, 21 Dec 2019 17:23:24 +0000
> > From: Alan Mackenzie <address@hidden>
> >
> > /* FIXME: Call signal_after_change! */
> > + beg = PT;
> Can you tell why you needed this variable and the assignment? AFAIK,
> PT doesn't change when we call decode_coding_c_string.
I'd misunderstood decode_coding_c_string. You're right, PT doesn't
change with this macro. With this, the arguments I was getting to
after-change-functions were wrong.
However, I've kept BEG for the next version of the patch.
> > decode_coding_c_string (&process_coding,
> > (unsigned char *) buf, nread, curbuf);
> > unbind_to (count1, Qnil);
> > + signal_after_change (beg, 0, PT - beg);
> > if (display_on_the_fly
> > && CODING_REQUIRE_DETECTION (&saved_coding)
> > && ! CODING_REQUIRE_DETECTION (&process_coding))
> >
> >
> > , and this appears to solve the OP's problem.
> >
> > However, a few lines further on, there's a del_range_2 call inside a
> > condition
> > I don't understand (though might, with a great deal of study). I suspect
> > that
> > the call to signal_after_change ought to take this del_range_2 into account,
> > possibly coming after it.
> >
> > Would somebody who's familiar with this bit of callproc.c please help me out
> > here, and explain what this call to del_range_2 is doing, and whether
> > there's
> > anything basically wrong with my simple-minded addition of
> > signal_after_change.
> I'm not sure what you want to hear. The del_range_2 call deletes the
> just-inserted text, because the condition means that text was inserted
> using the wrong coding-system to decode the incoming bytes. What does
> that mean for the modification hooks, I don't know: the
> before-change-functions were already called, but nothing was inserted
> from the Lisp application's POV, so if you insist on having before and
> after hooks to be called in pairs, you are in a conundrum.
I think I've solved this with the new variable prepared_position. It
records the position of BEG for prepare_to_modify_buffer, and only calls
that function if it hasn't already done so for that BEG. It's not an
elegant solution, but I think it will work.
> It's possible that we should simplify all this by calling the before
> hooks just once before the loop and the after hooks just once after
> the loop, instead of calling them for each individual chunk inside the
> loop, but again I don't know what that means for applications which
> expects these hook calls to pair.
I don't think this is necessary. (If we positively want to do it,
that's a different matter.)
Here's the latest version of my patch. It's only been slightly tested,
but it doesn't produce any unexpected "successes" from 'make check'.
diff --git a/src/callproc.c b/src/callproc.c
index b51594c2d5..34da7af863 100644
--- a/src/callproc.c
+++ b/src/callproc.c
@@ -746,6 +746,8 @@ call_process (ptrdiff_t nargs, Lisp_Object *args, int
filefd,
int carryover = 0;
bool display_on_the_fly = display_p;
struct coding_system saved_coding = process_coding;
+ ptrdiff_t prepared_position = 0;
+ ptrdiff_t beg;
while (1)
{
@@ -780,7 +782,13 @@ call_process (ptrdiff_t nargs, Lisp_Object *args, int
filefd,
;
else if (NILP (BVAR (current_buffer, enable_multibyte_characters))
&& ! CODING_MAY_REQUIRE_DECODING (&process_coding))
- insert_1_both (buf, nread, nread, 0, 1, 0);
+ {
+ beg = PT;
+ insert_1_both (buf, nread, nread, 0, prepared_position < PT, 0);
+ if (prepared_position < PT)
+ prepared_position = PT;
+ signal_after_change (beg, 0, PT - beg);
+ }
else
{ /* We have to decode the input. */
Lisp_Object curbuf;
@@ -788,7 +796,11 @@ call_process (ptrdiff_t nargs, Lisp_Object *args, int
filefd,
XSETBUFFER (curbuf, current_buffer);
/* FIXME: Call signal_after_change! */
- prepare_to_modify_buffer (PT, PT, NULL);
+ if (prepared_position < PT)
+ {
+ prepare_to_modify_buffer (PT, PT, NULL);
+ prepared_position = PT;
+ }
/* We cannot allow after-change-functions be run
during decoding, because that might modify the
buffer, while we rely on process_coding.produced to
@@ -822,6 +834,7 @@ call_process (ptrdiff_t nargs, Lisp_Object *args, int
filefd,
continue;
}
+ signal_after_change (PT, 0, process_coding.produced_char);
TEMP_SET_PT_BOTH (PT + process_coding.produced_char,
PT_BYTE + process_coding.produced);
carryover = process_coding.carryover_bytes;
--
Alan Mackenzie (Nuremberg, Germany).
- /* FIXME: Call signal_after_change! */ in callproc.c. Well, why not?, Alan Mackenzie, 2019/12/21
- Re: /* FIXME: Call signal_after_change! */ in callproc.c. Well, why not?, Eli Zaretskii, 2019/12/21
- 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, 2019/12/22
- Re: /* FIXME: Call signal_after_change! */ in callproc.c. Well, why not?, Alan Mackenzie, 2019/12/24
- Re: /* FIXME: Call signal_after_change! */ in callproc.c. Well, why not?, Alan Mackenzie, 2019/12/24
- Re: /* FIXME: Call signal_after_change! */ in callproc.c. Well, why not?, Eli Zaretskii, 2019/12/24
- Re: /* FIXME: Call signal_after_change! */ in callproc.c. Well, why not?, Eli Zaretskii, 2019/12/24
- Re: /* FIXME: Call signal_after_change! */ in callproc.c. Well, why not?, Alan Mackenzie, 2019/12/29
- Re: /* FIXME: Call signal_after_change! */ in callproc.c. Well, why not?, Stefan Monnier, 2019/12/29