Re: [Emacs-diffs] fix/no-undo-boundary-on-secondary-buffer-change c7a6601 1/5: undo-size can count number of boundaries.

[Phillip Lord]

Stefan Monnier <address@hidden> writes:
> Here are some nitpicks and suggestions on your code.
>
>> +  if(n){
>
> Please try to reproduce the code layout you see around.
>
>> +          // we have a boundary, so check we do not have too many
>
> Please capitalize and punctuate your comments.
>
>> +          if (XCAR (elt) == Qnil)
>
> Don't compare Lisp_Objects with == but with "EQ (x, y)" or in this case
> with "NILP (elt)".  While hacking on the C code I recommend you
>
>    ./configure --enable-checking > (and/or (re)read the GNU Coding Standards 
> which describe the coding
> style to use).
>
> In the above:
> - Add a space before the open paren.
> - Please the open brace on its own line.
--enable-check-lisp-object-type
>
> which will help you catch those problems and a few others.

All those should be fixed now.

>
>> +  if(NILP (Vundo_buffer_undoably_changed)){
>> +    Fset (Qundo_buffer_undoably_changed,Qt);
>> +    safe_run_hooks (Qundo_first_undoable_change_hook);
>> +  }
>
> Why do you need Vundo_buffer_undoably_changed?
> Doesn't (car-safe buffer-undo-list) give you the same information?

No. The point is that this can be reset when ever I choose, and so it
may well occur *not* after a boundary.

Still your question tells me that my name is not very good.
"first-recent-change" hook might be better, and
"buffer-recently-changed", perhaps.

You should be able to see how this all fits together now. Part of my
purpose is that the meaning and semantics of "recent" are now in the
hands of the lisp developer, since they can reset
"buffer-unably-changed" as they want, which will result in a new call to
the hook.

I have chosen not to add a "undoable-change" hook, because I didn't need
it. The first change hook calls rarely, so the performance issues with
handling this change in lisp are, I think, unimportant.



>> +               doc: /* Normal hook run when a buffer has its first recent 
>> undo-able change.
>
> Rather than "recent undo-able change", I'd talk about the "first undo
> record after a boundary".

I would have, iff it did this.

>> +This hook will be run with `current-buffer' as the buffer that
>> +has changed. Recent means since the value of `undo-buffer-undoably-changed'
>               ^^^
> Please try and follow the sentence-end-double-space convention.

Fixed now, hopefully.

Phil