Undo-limit default

Undo-limit default

[Chong Yidong]

A user reported bug#1501 after inadvertently encountering the undo
limit.  The default is 20kb.  Does anyone on this list find this too
low, considering how much memory computers possess these days?

(I personally don't feel strongly about this either way, but I can see
how it can be surprising to users.  These days, other programs seem to
store much more undo information.)

Re: Undo-limit default

[Michael Ekstrand]

[-- Attachment #1: Type: text/plain, Size: 1177 bytes --]

Chong Yidong <cyd@stupidchicken.com> writes:
> A user reported bug#1501 after inadvertently encountering the undo
> limit.  The default is 20kb.  Does anyone on this list find this too
> low, considering how much memory computers possess these days?

This is totally subjective, but I seem to have a vague memory of running
in to the undo limit sometime in the last few months.  It doesn't happen
very often, though, and in cases where I'm likely to use that much undo
I'm just as likely to do a VCS revert.

> (I personally don't feel strongly about this either way, but I can see
> how it can be surprising to users.  These days, other programs seem to
> store much more undo information.)

Increasing the default a bit probably can't hurt IMO.  Out of curiosity,
is this limit global or per-buffer?  The documentation for `undo-limit'
doesn't say, and the relevant section in the Emacs manual doesn't
clearly state it either.

- Michael

-- 
mouse, n: A device for pointing at the xterm in which you want to type.
Confused by the strange files?  I cryptographically sign my messages.
For more information see <http://www.elehack.net/resources/gpg>.

[-- Attachment #2: Type: application/pgp-signature, Size: 196 bytes --]

Re: Undo-limit default

[Lennart Borgman]

On Sun, Dec 7, 2008 at 2:35 AM, Chong Yidong <cyd@stupidchicken.com> wrote:
> A user reported bug#1501 after inadvertently encountering the undo
> limit.  The default is 20kb.  Does anyone on this list find this too
> low, considering how much memory computers possess these days?

How much memory does that consume?

> (I personally don't feel strongly about this either way, but I can see
> how it can be surprising to users.  These days, other programs seem to
> store much more undo information.)

Emacs should be best, shouldn't it? Seriously, if it is surprising to
users isn't it to low then?

Re: Undo-limit default

[Dan Nicolaescu]

Chong Yidong <cyd@stupidchicken.com> writes:

  > A user reported bug#1501 after inadvertently encountering the undo
  > limit.  The default is 20kb.  Does anyone on this list find this too
  > low, considering how much memory computers possess these days?

I do, I had to increase it in my .emacs a while ago because I kept
hitting the limit.  But I don't have any suggestion what a good value
would be.

Re: Undo-limit default

[martin rudalics]

 > A user reported bug#1501 after inadvertently encountering the undo
 > limit.  The default is 20kb.  Does anyone on this list find this too
 > low, considering how much memory computers possess these days?

For some years I use

  '(undo-limit 200000)
  '(undo-outer-limit 300000)
  '(undo-strong-limit 300000)

but I don't understand why we keep font-lock assigned properties (in
particular the completely useless 'fontified property) on the undo-list.
Here these usually account for more than 90% of all entries.

martin

Re: Undo-limit default

[Lennart Borgman]

On Sun, Dec 7, 2008 at 2:02 PM, martin rudalics <rudalics@gmx.at> wrote:
> but I don't understand why we keep font-lock assigned properties (in
> particular the completely useless 'fontified property) on the undo-list.
> Here these usually account for more than 90% of all entries.

I thought that the `fontified' property did not go into the undo-list.
Are not the calls changing that property surrounded by macros that
should prevent that?

What major modes do you see this in?

Re: Undo-limit default

[martin rudalics]

 > I thought that the `fontified' property did not go into the undo-list.
 > Are not the calls changing that property surrounded by macros that
 > should prevent that?

Only those of font-lock itself.

 > What major modes do you see this in?

All font-locked ones.  The property is stored as a text property and
shows up whenever you delete or reinsert corresponding text.

IMHO putting any font-lock handled property on the undo-list is harmful.

martin

Re: Undo-limit default

[Chong Yidong]

martin rudalics <rudalics@gmx.at> writes:

>> I thought that the `fontified' property did not go into the undo-list.
>> Are not the calls changing that property surrounded by macros that
>> should prevent that?
>
> Only those of font-lock itself.
>
>> What major modes do you see this in?
>
> All font-locked ones.  The property is stored as a text property and
> shows up whenever you delete or reinsert corresponding text.
>
> IMHO putting any font-lock handled property on the undo-list is harmful.

Could you check whether stripping out the font-lock information has any
bad effects?  The following untested patch should implement this
behavior:

*** trunk/src/undo.c.~1.86.~	2008-09-22 11:47:50.000000000 -0400
--- trunk/src/undo.c	2008-12-07 09:25:53.000000000 -0500
***************
*** 60,65 ****
--- 60,69 ----
  
  int undo_inhibit_record_point;
  
+ /* Text properties not recorded in the undo list.  */
+ 
+ Lisp_Object Vundo_discarded_text_properties;
+ 
  /* Record point as it was at beginning of this command (if necessary)
     and prepare the undo info for recording a change.
     PT is the position of point that will naturally occur as a result of the
***************
*** 171,176 ****
--- 175,184 ----
  {
    Lisp_Object sbeg;
  
+   string = Fremove_text_properties (0, SCHARS (string),
+ 				    Vundo_discarded_text_properties,
+ 				    string);
+ 
    if (EQ (current_buffer->undo_list, Qt))
      return;
  
***************
*** 734,739 ****
--- 742,756 ----
    DEFVAR_BOOL ("undo-inhibit-record-point", &undo_inhibit_record_point,
  	       doc: /* Non-nil means do not record `point' in `buffer-undo-list'.  */);
    undo_inhibit_record_point = 0;
+ 
+   DEFVAR_INT ("undo-discarded-text-properties", &Vundo_discarded_text_properties,
+ 	      doc: /* Text properties not recorded in the undo list.
+ This should be a property list whose property names specify the
+ properties to remove.  (The values stored in the property list are
+ ignored.)  */);
+   Vundo_discarded_text_properties = list4 (intern("fontified nil"), Qnil,
+ 					   intern ("jit-lock-defer-multiline"), Qnil);
+   staticpro (&Vundo_discarded_text_properties);
  }
  
  /* arch-tag: d546ee01-4aed-4ffb-bb8b-eefaae50d38a

Re: Undo-limit default

[Chong Yidong]

Chong Yidong <cyd@stupidchicken.com> writes:

>> IMHO putting any font-lock handled property on the undo-list is harmful.
>
> Could you check whether stripping out the font-lock information has any
> bad effects?  The following untested patch should implement this
> behavior

BTW, I don't think such a change should go into Emacs 23.1; it might be
worth doing after the release.

Re: Undo-limit default

[Richard M Stallman]

    > Could you check whether stripping out the font-lock information has any
    > bad effects?  The following untested patch should implement this
    > behavior

    BTW, I don't think such a change should go into Emacs 23.1; it might be
    worth doing after the release.

That depends on how much it affects performance.  I have noticed
lots of garbage collection during font lock processing.
Is this the reason why?  Does fixing this lead to a big speedup?

Re: Undo-limit default

[martin rudalics]

 > Could you check whether stripping out the font-lock information has any
 > bad effects?  The following untested patch should implement this
 > behavior:

Crashes Emacs at the time I delete text in a buffer.  I can look into
this later.

BTW, I suppose you mean that font-lock should add all face and extra
properties too when it's active in a buffer.

martin

Re: Undo-limit default

[Chong Yidong]

Chong Yidong <cyd@stupidchicken.com> writes:

> Could you check whether stripping out the font-lock information has any
> bad effects?  The following untested patch should implement this
> behavior:

Whoops, too untested :-P

Try this patch instead:

*** trunk/src/undo.c.~1.86.~	2008-09-24 23:23:04.000000000 -0400
--- trunk/src/undo.c	2008-12-07 10:05:52.000000000 -0500
***************
*** 60,65 ****
--- 60,69 ----
  
  int undo_inhibit_record_point;
  
+ /* Text properties not recorded in the undo list.  */
+ 
+ Lisp_Object Vundo_discarded_text_properties;
+ 
  /* Record point as it was at beginning of this command (if necessary)
     and prepare the undo info for recording a change.
     PT is the position of point that will naturally occur as a result of the
***************
*** 174,179 ****
--- 178,187 ----
    if (EQ (current_buffer->undo_list, Qt))
      return;
  
+   Fremove_text_properties (make_number (0), make_number (SCHARS (string)),
+ 			   Vundo_discarded_text_properties,
+ 			   string);
+ 
    if (PT == beg + SCHARS (string))
      {
        XSETINT (sbeg, -beg);
***************
*** 734,739 ****
--- 742,757 ----
    DEFVAR_BOOL ("undo-inhibit-record-point", &undo_inhibit_record_point,
  	       doc: /* Non-nil means do not record `point' in `buffer-undo-list'.  */);
    undo_inhibit_record_point = 0;
+ 
+   DEFVAR_LISP ("undo-discarded-text-properties", &Vundo_discarded_text_properties,
+ 	       doc: /* Text properties not recorded in the undo list.
+ This should be a property list whose property names specify the
+ properties to remove.  (The values stored in the property list are
+ ignored.)  */);
+   Vundo_discarded_text_properties
+     = list4 (intern ("fontified"), Qnil,
+ 	     intern ("jit-lock-defer-multiline"), Qnil);
+   staticpro (&Vundo_discarded_text_properties);
  }
  
  /* arch-tag: d546ee01-4aed-4ffb-bb8b-eefaae50d38a

Re: Undo-limit default

[martin rudalics]

 > Try this patch instead:

This works.  I can do

     (set (make-local-variable 'undo-discarded-text-properties)
	 (append undo-discarded-text-properties
		 (if font-lock-syntactic-keywords
		     '(syntax-table nil face nil font-lock-multiline nil)
		   '(face nil font-lock-multiline nil))))

in `font-lock-set-defaults'.

Thanks, martin.

Re: Undo-limit default

[Stefan Monnier]

> +   string = Fremove_text_properties (0, SCHARS (string),
> + 				    Vundo_discarded_text_properties,
> + 				    string);

I wonder:
- Why use an alist rather than a list.
- I.e. why use Fremove_text_properties rather than
  Fremove_list_of_text_properties?
- Why isn't one of those two functions implemented by calling the other one?
- And why is the code of the two functions different?


        Stefan

Re: Undo-limit default

[Richard M Stallman]

    - Why use an alist rather than a list.

I think Joe Arceneaux did it that way just to have parallel calling
conventions for several functions.

Someone later added `remove-list-of-text-properties' which is similar but
takes a list of property names.

It would be possible to implement `remove-list-of-text-properties' by
consing up an alist and calling `remove-text-properties', but that
would be wasteful consing.  The inverse would also be wasteful
consing.

Re: Undo-limit default

[Stefan Monnier]

>     - Why use an alist rather than a list.
> I think Joe Arceneaux did it that way just to have parallel calling
> conventions for several functions.

I meant in the new code, not in remove-text-properties.

> It would be possible to implement `remove-list-of-text-properties' by
> consing up an alist and calling `remove-text-properties', but that
> would be wasteful consing.  The inverse would also be wasteful
> consing.

The current duplication of code (worse yet: it's not actually identical)
is a problem.  I'd rather have some extra consing (and just mark the
consing one as obsolete).  And I'm pretty sure we can also remove the
duplication without adding extra consing.


        Stefan