* 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).