* undo refactoring
[not found] ` <jwv60sl85nl.fsf-monnier+Inbox@gnu.org>
@ 2016-07-05 8:46 ` Phillip Lord
2016-07-05 21:50 ` Stefan Monnier
0 siblings, 1 reply; 3+ messages in thread
From: Phillip Lord @ 2016-07-05 8:46 UTC (permalink / raw)
To: Stefan Monnier; +Cc: Emacs-Devel devel
I thought to move this to emacs-devel rather than #23781 since it's not
directly relevant.
I'll think about the FIXME, that I've just added to undo.c, and see if I
can update that on master. But also, while reading the code again, I've
realised that I don't understand the reason for pre-allocating the
boundary, since when it actually used we do this:
/* One way or another, cons nil onto the front of the undo list. */
if (!NILP (pending_boundary))
{
/* If we have preallocated the cons cell to use here,
use that one. */
XSETCDR (pending_boundary, BVAR (current_buffer, undo_list));
bset_undo_list (current_buffer, pending_boundary);
pending_boundary = Qnil;
}
else
bset_undo_list (current_buffer,
Fcons (Qnil, BVAR (current_buffer, undo_list)));
Why not just do
bset_undo_list (current_buffer,
Fcons (Qnil, BVAR (current_buffer, undo_list)));
and remove all references to pending_boundary (and therefore
`prepare_record` which is now rather badly named)? Or am I missing
something
Phil
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: undo refactoring
2016-07-05 8:46 ` undo refactoring Phillip Lord
@ 2016-07-05 21:50 ` Stefan Monnier
2016-07-05 22:22 ` Phillip Lord
0 siblings, 1 reply; 3+ messages in thread
From: Stefan Monnier @ 2016-07-05 21:50 UTC (permalink / raw)
To: Phillip Lord; +Cc: Emacs-Devel devel
> Why not just do
> bset_undo_list (current_buffer,
> Fcons (Qnil, BVAR (current_buffer, undo_list)));
> and remove all references to pending_boundary (and therefore
> `prepare_record` which is now rather badly named)? Or am I missing
> something
IIUC, the reason for this was so as to avoid an inf-loop when malloc
fails (for lack of memory).
The particular issue was that it was OK for a command to fail because of
malloc failure, but it was not OK for the read-eval-loop itself to fail
before having the opportunity to run another command.
My impression is that this design goal has been ignored for much too
long, so I don't think Emacs behaves this way any more. And instead we
have now other ways to handle the memory-full situation (such as the
extra memory that's pre-allocated and then released in case the memory
is full).
IOW, I'd be surprised if getting rid of this quirk would ever lead to
a visible change in Emacs's behavior nowadays.
Stefan
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: undo refactoring
2016-07-05 21:50 ` Stefan Monnier
@ 2016-07-05 22:22 ` Phillip Lord
0 siblings, 0 replies; 3+ messages in thread
From: Phillip Lord @ 2016-07-05 22:22 UTC (permalink / raw)
To: Stefan Monnier; +Cc: Emacs-Devel devel
Stefan Monnier <monnier@iro.umontreal.ca> writes:
> IIUC, the reason for this was so as to avoid an inf-loop when malloc
> fails (for lack of memory).
>
> The particular issue was that it was OK for a command to fail because of
> malloc failure, but it was not OK for the read-eval-loop itself to fail
> before having the opportunity to run another command.
Found the documentation (near the variable rather than the method), and
yes, it's about memory allocation.
/* The first time a command records something for undo.
it also allocates the undo-boundary object
which will be added to the list at the end of the command.
This ensures we can't run out of space while trying to make
an undo-boundary. */
That comment was from Richard, and hasn't changed since 1994; I guess
the chance of memory issues from a cons or two are less of a problem
now.
>
> My impression is that this design goal has been ignored for much too
> long, so I don't think Emacs behaves this way any more. And instead we
> have now other ways to handle the memory-full situation (such as the
> extra memory that's pre-allocated and then released in case the memory
> is full).
>
> IOW, I'd be surprised if getting rid of this quirk would ever lead to
> a visible change in Emacs's behavior nowadays.
I've experimented with this and, at least, all the tests work. I'll try
it out in actual use, then if you are happy make the change on master.
Phil
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-07-05 22:22 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <m2mvm3fz14.fsf@metalevel.at>
[not found] ` <83h9cavdgj.fsf@gnu.org>
[not found] ` <87poqyy2tc.fsf@metalevel.at>
[not found] ` <87vb0qqrkz.fsf@russet.org.uk>
[not found] ` <87h9c9zx75.fsf@metalevel.at>
[not found] ` <834m89vmyv.fsf@gnu.org>
[not found] ` <878txlsbdb.fsf@russet.org.uk>
[not found] ` <87furtccdv.fsf@metalevel.at>
[not found] ` <877fd5q9te.fsf@russet.org.uk>
[not found] ` <83bn2gtruk.fsf@gnu.org>
[not found] ` <87k2h37pvb.fsf@russet.org.uk>
[not found] ` <jwv37nqa0dc.fsf-monnier+bug#23871@gnu.org>
[not found] ` <87vb0lta67.fsf@russet.org.uk>
[not found] ` <jwv60sl85nl.fsf-monnier+Inbox@gnu.org>
2016-07-05 8:46 ` undo refactoring Phillip Lord
2016-07-05 21:50 ` Stefan Monnier
2016-07-05 22:22 ` Phillip Lord
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).