* [Emacs-diffs] master fe3676f: (Finsert_file_contents): Keep buffer consistent in non-local exit
@ 2019-07-03 6:39 Eli Zaretskii
2019-07-03 16:43 ` Stefan Monnier
0 siblings, 1 reply; 5+ messages in thread
From: Eli Zaretskii @ 2019-07-03 6:39 UTC (permalink / raw)
To: Stefan Monnier; +Cc: emacs-devel
Is it right to remove decode_coding_unwind? If set-auto-coding or
find-operation-coding-system signal an error, we will now:
. leave the buffer unibyte and with undo-list bound to t
. fail to reset the markers and overlays to their previous state
. fail to reset intervals
Why is that TRT? If there's something in your recent changes that
makes those repairs unnecessary, can you point me to what I missed?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Emacs-diffs] master fe3676f: (Finsert_file_contents): Keep buffer consistent in non-local exit
2019-07-03 6:39 [Emacs-diffs] master fe3676f: (Finsert_file_contents): Keep buffer consistent in non-local exit Eli Zaretskii
@ 2019-07-03 16:43 ` Stefan Monnier
2019-07-03 17:10 ` Eli Zaretskii
0 siblings, 1 reply; 5+ messages in thread
From: Stefan Monnier @ 2019-07-03 16:43 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: emacs-devel
> Is it right to remove decode_coding_unwind? If set-auto-coding or
> find-operation-coding-system signal an error, we will now:
>
> . leave the buffer unibyte and with undo-list bound to t
Since the error happens before decoding and even before we have chosen
a coding-system, there aren't many choices in this respect:
1- do as I do now (i.e. stay in unibyte even if the buffer was
originally in multibyte).
2- erase the text we loaded (i.e. all the text, since the buffer
started as empty) and revert the buffer to its original multibyteness
state.
3- decode the text with some "default" coding system. Note that the
behavior (1) basically corresponds to a `no-conversion` coding system
(since such a coding system also forces the buffer to unibyte).
4- keep the old "arbitrary bytes in a multibyte buffer" bug.
I think only (4) is clearly inferior and neither of the other is clearly
superior, so I went with the easier choice, especially since it's
expected to be a very rare corner case occurrence anyway (after all, we
lived with (4) for many years).
W.r.t the undo-list, I don't have a good reason for this choice, it was
just easier. Since the buffer started as empty, there's a good chance
that the undo list had no interesting info anyway, so I felt like this
is even less important.
> . fail to reset the markers and overlays to their previous state
> . fail to reset intervals
These only need to be "reset" in the normal code path because we remove
the text from the buffer (to put it into the gap and then decode it),
but since in this case we don't remove the text, there's no need to do
that (and remember that the buffer was empty to start with so there's
not much "previous state" to restore in this respect).
Stefan
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Emacs-diffs] master fe3676f: (Finsert_file_contents): Keep buffer consistent in non-local exit
2019-07-03 16:43 ` Stefan Monnier
@ 2019-07-03 17:10 ` Eli Zaretskii
2019-07-03 17:48 ` Stefan Monnier
0 siblings, 1 reply; 5+ messages in thread
From: Eli Zaretskii @ 2019-07-03 17:10 UTC (permalink / raw)
To: Stefan Monnier; +Cc: emacs-devel
> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org
> Date: Wed, 03 Jul 2019 12:43:10 -0400
>
> 1- do as I do now (i.e. stay in unibyte even if the buffer was
> originally in multibyte).
> 2- erase the text we loaded (i.e. all the text, since the buffer
> started as empty) and revert the buffer to its original multibyteness
> state.
> 3- decode the text with some "default" coding system. Note that the
> behavior (1) basically corresponds to a `no-conversion` coding system
> (since such a coding system also forces the buffer to unibyte).
> 4- keep the old "arbitrary bytes in a multibyte buffer" bug.
>
> I think only (4) is clearly inferior and neither of the other is clearly
> superior, so I went with the easier choice, especially since it's
> expected to be a very rare corner case occurrence anyway (after all, we
> lived with (4) for many years).
I think we should do 2.
> W.r.t the undo-list, I don't have a good reason for this choice, it was
> just easier. Since the buffer started as empty, there's a good chance
> that the undo list had no interesting info anyway, so I felt like this
> is even less important.
I think we should restore undo-list, because (a) it isn't hard, and
(b) someone at some point will complain if we don't.
> > . fail to reset the markers and overlays to their previous state
> > . fail to reset intervals
>
> These only need to be "reset" in the normal code path because we remove
> the text from the buffer (to put it into the gap and then decode it),
> but since in this case we don't remove the text, there's no need to do
> that (and remember that the buffer was empty to start with so there's
> not much "previous state" to restore in this respect).
I don't think I understand this. Are you saying that empty buffers
cannot have markers and overlays?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Emacs-diffs] master fe3676f: (Finsert_file_contents): Keep buffer consistent in non-local exit
2019-07-03 17:10 ` Eli Zaretskii
@ 2019-07-03 17:48 ` Stefan Monnier
2019-07-04 13:28 ` Eli Zaretskii
0 siblings, 1 reply; 5+ messages in thread
From: Stefan Monnier @ 2019-07-03 17:48 UTC (permalink / raw)
To: emacs-devel
>> 1- do as I do now (i.e. stay in unibyte even if the buffer was
>> originally in multibyte).
>> 2- erase the text we loaded (i.e. all the text, since the buffer
>> started as empty) and revert the buffer to its original multibyteness
>> state.
>> 3- decode the text with some "default" coding system. Note that the
>> behavior (1) basically corresponds to a `no-conversion` coding system
>> (since such a coding system also forces the buffer to unibyte).
>> 4- keep the old "arbitrary bytes in a multibyte buffer" bug.
>>
>> I think only (4) is clearly inferior and neither of the other is clearly
>> superior, so I went with the easier choice, especially since it's
>> expected to be a very rare corner case occurrence anyway (after all, we
>> lived with (4) for many years).
>
> I think we should do 2.
I think as a user, after an error, I'd rather see the text decoded with
`raw-text-unix` (which is what we get now) than no text at all.
Also until now, the behavior has been to show the text we loaded, so
this would be a more significant change.
What would be the benefit of making the buffer empty?
>> W.r.t the undo-list, I don't have a good reason for this choice, it was
>> just easier. Since the buffer started as empty, there's a good chance
>> that the undo list had no interesting info anyway, so I felt like this
>> is even less important.
>
> I think we should restore undo-list, because (a) it isn't hard, and
> (b) someone at some point will complain if we don't.
Fair enough, I'll do that.
>> > . fail to reset the markers and overlays to their previous state
>> > . fail to reset intervals
>>
>> These only need to be "reset" in the normal code path because we remove
>> the text from the buffer (to put it into the gap and then decode it),
>> but since in this case we don't remove the text, there's no need to do
>> that (and remember that the buffer was empty to start with so there's
>> not much "previous state" to restore in this respect).
> I don't think I understand this. Are you saying that empty buffers
> cannot have markers and overlays?
No I'm saying that all the markers were at BOB and it's perfectly
correct to have them moved elsewhere after the insertion of text in
the buffer.
The resetting of markers in the normal codepath is required for
correctness reasons (without it, we can end up with markers pointing
outside of the actual buffer text, for example), but in the non-local
exit case it's not needed and would only throw away information.
But if you prefer, I can move that back to the unwind function (along
with the undo-list resetting), of course.
Stefan
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Emacs-diffs] master fe3676f: (Finsert_file_contents): Keep buffer consistent in non-local exit
2019-07-03 17:48 ` Stefan Monnier
@ 2019-07-04 13:28 ` Eli Zaretskii
0 siblings, 0 replies; 5+ messages in thread
From: Eli Zaretskii @ 2019-07-04 13:28 UTC (permalink / raw)
To: Stefan Monnier; +Cc: emacs-devel
> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Wed, 03 Jul 2019 13:48:27 -0400
>
> > I think we should do 2.
>
> I think as a user, after an error, I'd rather see the text decoded with
> `raw-text-unix` (which is what we get now) than no text at all.
> Also until now, the behavior has been to show the text we loaded, so
> this would be a more significant change.
>
> What would be the benefit of making the buffer empty?
I think our usual practice is to roll back any failed operations,
leaving things as they were before the problematic transaction
started.
> > I think we should restore undo-list, because (a) it isn't hard, and
> > (b) someone at some point will complain if we don't.
>
> Fair enough, I'll do that.
>
> >> > . fail to reset the markers and overlays to their previous state
> >> > . fail to reset intervals
> >>
> >> These only need to be "reset" in the normal code path because we remove
> >> the text from the buffer (to put it into the gap and then decode it),
> >> but since in this case we don't remove the text, there's no need to do
> >> that (and remember that the buffer was empty to start with so there's
> >> not much "previous state" to restore in this respect).
> > I don't think I understand this. Are you saying that empty buffers
> > cannot have markers and overlays?
>
> No I'm saying that all the markers were at BOB and it's perfectly
> correct to have them moved elsewhere after the insertion of text in
> the buffer.
>
> The resetting of markers in the normal codepath is required for
> correctness reasons (without it, we can end up with markers pointing
> outside of the actual buffer text, for example), but in the non-local
> exit case it's not needed and would only throw away information.
>
> But if you prefer, I can move that back to the unwind function (along
> with the undo-list resetting), of course.
To reset the undo-list, you already need an unwind function, right?
If you are sure that markers and overlays will not be affected by
insertion that was aborted half-way, then the unwind function doesn't
need to do anything with them. I couldn't convince myself that none
of these will be affected.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-07-04 13:28 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-07-03 6:39 [Emacs-diffs] master fe3676f: (Finsert_file_contents): Keep buffer consistent in non-local exit Eli Zaretskii
2019-07-03 16:43 ` Stefan Monnier
2019-07-03 17:10 ` Eli Zaretskii
2019-07-03 17:48 ` Stefan Monnier
2019-07-04 13:28 ` Eli Zaretskii
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.