Re: jit lock sit-for provokes redisplay provokes imenu

Re: jit lock sit-for provokes redisplay provokes imenu

[Richard Stallman]

    After ~30s of idle time, I see imenu-update-menubar being called
    continuously by emacs.  At a guess, imenu is being triggered because
    jit-lock has kicked in.

That seems plausible.  Jit-lock does fontification by adding text
properties, and that modifies the buffer, so that the subsequent
redisplay will call imenu-update-menubar.

The obvious way to solve this is to add a feature that enables
imenu-update-menubar to determine that there have been no changes
except in text properties.  Then it could return without really doing
anything.

It could be a function buffer-chars-modified-tick,
which records the value of buffer-modified-tick as of the last
change in the characters in the buffer.

Would someone like to implement this?

RE: jit lock sit-for provokes redisplay provokes imenu

[Drew Adams]

        After ~30s of idle time, I see imenu-update-menubar being called
        continuously by emacs.  At a guess, imenu is being triggered because
        jit-lock has kicked in.

    That seems plausible.  Jit-lock does fontification by adding text
    properties, and that modifies the buffer, so that the subsequent
    redisplay will call imenu-update-menubar.

    The obvious way to solve this is to add a feature that enables
    imenu-update-menubar to determine that there have been no changes
    except in text properties.  Then it could return without really doing
    anything.

    It could be a function buffer-chars-modified-tick,
    which records the value of buffer-modified-tick as of the last
    change in the characters in the buffer.

I don't really mean to meddle here, but we talked about distinguishing text
changes from things like text-properties changes, for purposes of undo and
indicating buffer modification in the mode line - after the release.

It sounds like this problem could perhaps reuse a predicate that tells
whether buffer text was modified (as opposed to just text properties), once
that is implemented (after the release).

If so, you might want to make a note of that, and revisit the fix to this
problem later, so it uses the generic predicate.

Re: jit lock sit-for provokes redisplay provokes imenu

[Richard Stallman]

	It could be a function buffer-chars-modified-tick,
	which records the value of buffer-modified-tick as of the last
	change in the characters in the buffer.

    I don't really mean to meddle here, but we talked about distinguishing text
    changes from things like text-properties changes, for purposes of undo and
    indicating buffer modification in the mode line - after the release.

This change, with the design I proposed, is so simple that it can't
break anything.  It would enable us to get rid of a gross
inefficiency, which is a kind of bug.  So I think we should do it now.

Would someone like to implement it?

Re: jit lock sit-for provokes redisplay provokes imenu

[martin rudalics]

 > This change, with the design I proposed, is so simple that it can't
 > break anything.  It would enable us to get rid of a gross
 > inefficiency, which is a kind of bug.  So I think we should do it now.

The following questions would have to be answered before doing that:

1. Should the value returned by `buffer-chars-modified-tick' always
(invariantly) imply the value returned by `buffer-modified-tick'?  With
other words, suppose we have a thing called CHAR_MODIFF: Would we have
to support the invariant

		  MODIFF >= CHAR_MODIFF >= SAVE_MODIFF

In my opinion it seems counterintuitive to say that a buffer is "not
modified" but a "character insertion and/or deletion has occurred".
Hence I think the answer is yes.

Affected seem (at least): `write-region', `insert-file-contents',
`set-buffer-modified-p' and `restore-buffer-modified-p'.  Any more?


2. Things like `subst-char-in-region' with NOUNDO non-nil change the
buffer but update the SAVE_MODIFF in order to simulate that no buffer
change occurred:

		  if (MODIFF - 1 == SAVE_MODIFF)
		    SAVE_MODIFF++;

I think that's innocuous but, if someone wanted to increment SAVE_MODIFF
in order to "ignore" a _text-property_ change only, the invariant above
could get violated.  Is anyone aware of whether such a hack exists or is
planned somewhere?


3. `first-change-hook' currently runs whenever a text-property is
assigned (by font-lock, for example).  I believe this should change with
the new function too?

Re: jit lock sit-for provokes redisplay provokes imenu

[martin rudalics]

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

 > It could be a function buffer-chars-modified-tick,
 > which records the value of buffer-modified-tick as of the last
 > change in the characters in the buffer.

Please check whether the attached patch removes the extra calls to
`imenu-update-menubar'.

[-- Attachment #2: modiff.patch --]
[-- Type: text/plain, Size: 8831 bytes --]

*** buffer.c	Tue Jun  6 19:20:40 2006
--- buffer.c	Fri Jul 21 16:00:22 2006
***************
*** 371,376 ****
--- 371,377 ----
    BUF_ZV_BYTE (b) = BEG_BYTE;
    BUF_Z_BYTE (b) = BEG_BYTE;
    BUF_MODIFF (b) = 1;
+   BUF_CHARS_MODIFF (b) = 1;
    BUF_OVERLAY_MODIFF (b) = 1;
    BUF_SAVE_MODIFF (b) = 1;
    BUF_INTERVALS (b) = 0;
***************
*** 1145,1150 ****
--- 1146,1172 ----

    return make_number (BUF_MODIFF (buf));
  }
+ 
+ DEFUN ("buffer-chars-modified-tick", Fbuffer_chars_modified_tick, Sbuffer_chars_modified_tick,
+        0, 1, 0,
+        doc: /* Return BUFFER's tick counter, incremented for each insert/delete.
+ Each buffer has a tick counter which is incremented each time text in
+ that buffer is inserted or deleted.  It wraps around occasionally.
+ No argument or nil as argument means use current buffer as BUFFER.  */)
+      (buffer)
+      register Lisp_Object buffer;
+ {
+   register struct buffer *buf;
+   if (NILP (buffer))
+     buf = current_buffer;
+   else
+     {
+       CHECK_BUFFER (buffer);
+       buf = XBUFFER (buffer);
+     }
+ 
+   return make_number (BUF_CHARS_MODIFF (buf));
+ }
  \f
  DEFUN ("rename-buffer", Frename_buffer, Srename_buffer, 1, 2,
         "sRename buffer (to new name): \nP",
***************
*** 6014,6019 ****
--- 6036,6042 ----
    defsubr (&Sbuffer_modified_p);
    defsubr (&Sset_buffer_modified_p);
    defsubr (&Sbuffer_modified_tick);
+   defsubr (&Sbuffer_chars_modified_tick);
    defsubr (&Srename_buffer);
    defsubr (&Sother_buffer);
    defsubr (&Sbuffer_enable_undo);

*** buffer.h	Tue Apr 11 16:24:24 2006
--- buffer.h	Fri Jul 21 15:31:34 2006
***************
*** 82,87 ****
--- 82,90 ----
  /* Modification count.  */
  #define MODIFF (current_buffer->text->modiff)

+ /* Character modification count.  */
+ #define CHARS_MODIFF (current_buffer->text->chars_modiff)
+ 
  /* Overlay modification count.  */
  #define OVERLAY_MODIFF (current_buffer->text->overlay_modiff)

***************
*** 147,152 ****
--- 150,158 ----
  /* Modification count.  */
  #define BUF_MODIFF(buf) ((buf)->text->modiff)

+ /* Character modification count.  */
+ #define BUF_CHARS_MODIFF(buf) ((buf)->text->chars_modiff)
+ 
  /* Modification count as of last visit or save.  */
  #define BUF_SAVE_MODIFF(buf) ((buf)->text->save_modiff)

***************
*** 406,411 ****
--- 412,421 ----
  				   for this buffer.  It is incremented for
  				   each such event, and never otherwise
  				   changed.  */
+     int chars_modiff;		/* This counts insert/delete events
+ 				   for this buffer.  It is incremented for
+ 				   each such event, and never otherwise
+ 				   changed.  */
      int save_modiff;		/* Previous value of modiff, as of last
  				   time buffer visited or saved a file.  */


*** casefiddle.c	Tue Apr 11 16:24:24 2006
--- casefiddle.c	Fri Jul 21 15:25:50 2006
***************
*** 189,194 ****
--- 189,195 ----
    validate_region (&b, &e);
    start = XFASTINT (b);
    end = XFASTINT (e);
+   CHARS_MODIFF++;
    modify_region (current_buffer, start, end);
    record_change (start, end - start);
    start_byte = CHAR_TO_BYTE (start);

*** editfns.c	Sun Jul  2 09:50:00 2006
--- editfns.c	Fri Jul 21 15:25:52 2006
***************
*** 2785,2790 ****
--- 2785,2791 ----
  	  if (! changed)
  	    {
  	      changed = pos;
+ 	      CHARS_MODIFF++;
  	      modify_region (current_buffer, changed, XINT (end));

  	      if (! NILP (noundo))
***************
*** 2897,2902 ****
--- 2898,2904 ----
    pos = XINT (start);
    pos_byte = CHAR_TO_BYTE (pos);
    end_pos = XINT (end);
+   CHARS_MODIFF++;
    modify_region (current_buffer, pos, XINT (end));

    cnt = 0;
***************
*** 4162,4167 ****
--- 4164,4170 ----

    if (end1 == start2)		/* adjacent regions */
      {
+       CHARS_MODIFF++;
        modify_region (current_buffer, start1, end2);
        record_change (start1, len1 + len2);

***************
*** 4218,4224 ****
          {
  	  USE_SAFE_ALLOCA;

!           modify_region (current_buffer, start1, end1);
            modify_region (current_buffer, start2, end2);
            record_change (start1, len1);
            record_change (start2, len2);
--- 4221,4228 ----
          {
  	  USE_SAFE_ALLOCA;

! 	  CHARS_MODIFF++;
! 	  modify_region (current_buffer, start1, end1);
            modify_region (current_buffer, start2, end2);
            record_change (start1, len1);
            record_change (start2, len2);
***************
*** 4248,4254 ****
          {
  	  USE_SAFE_ALLOCA;

!           modify_region (current_buffer, start1, end2);
            record_change (start1, (end2 - start1));
            tmp_interval1 = copy_intervals (cur_intv, start1, len1);
            tmp_interval_mid = copy_intervals (cur_intv, end1, len_mid);
--- 4252,4259 ----
          {
  	  USE_SAFE_ALLOCA;

! 	  CHARS_MODIFF++;
! 	  modify_region (current_buffer, start1, end2);
            record_change (start1, (end2 - start1));
            tmp_interval1 = copy_intervals (cur_intv, start1, len1);
            tmp_interval_mid = copy_intervals (cur_intv, end1, len_mid);
***************
*** 4279,4285 ****
  	  USE_SAFE_ALLOCA;

            record_change (start1, (end2 - start1));
!           modify_region (current_buffer, start1, end2);

            tmp_interval1 = copy_intervals (cur_intv, start1, len1);
            tmp_interval_mid = copy_intervals (cur_intv, end1, len_mid);
--- 4284,4291 ----
  	  USE_SAFE_ALLOCA;

            record_change (start1, (end2 - start1));
! 	  CHARS_MODIFF++;
! 	  modify_region (current_buffer, start1, end2);

            tmp_interval1 = copy_intervals (cur_intv, start1, len1);
            tmp_interval_mid = copy_intervals (cur_intv, end1, len_mid);

*** insdel.c	Tue Apr 11 16:24:24 2006
--- insdel.c	Fri Jul 21 15:25:58 2006
***************
*** 1007,1012 ****
--- 1007,1013 ----
       will add up to the right stuff in the undo list.  */
    record_insert (PT, nchars);
    MODIFF++;
+   CHARS_MODIFF++;

    bcopy (string, GPT_ADDR, nbytes);

***************
*** 1144,1149 ****
--- 1145,1151 ----

    record_insert (PT, nchars);
    MODIFF++;
+   CHARS_MODIFF++;

    GAP_SIZE -= outgoing_nbytes;
    GPT += nchars;
***************
*** 1295,1300 ****
--- 1297,1303 ----

    record_insert (PT, nchars);
    MODIFF++;
+   CHARS_MODIFF++;

    GAP_SIZE -= outgoing_nbytes;
    GPT += nchars;
***************
*** 1403,1408 ****
--- 1406,1412 ----
    if (len == 0)
      evaporate_overlays (from);
    MODIFF++;
+   CHARS_MODIFF++;
  }

  /* Like adjust_after_replace, but doesn't require PREV_TEXT.
***************
*** 1453,1458 ****
--- 1457,1463 ----
    if (len == 0)
      evaporate_overlays (from);
    MODIFF++;
+   CHARS_MODIFF++;
  }

  /* Record undo information, adjust markers and position keepers for an
***************
*** 1645,1650 ****
--- 1650,1656 ----
    CHECK_MARKERS ();

    MODIFF++;
+   CHARS_MODIFF++;
    UNGCPRO;

    signal_after_change (from, nchars_del, GPT - from);
***************
*** 1769,1774 ****
--- 1775,1781 ----
    CHECK_MARKERS ();

    MODIFF++;
+   CHARS_MODIFF++;
  }
  \f
  /* Delete characters in current buffer
***************
*** 1950,1955 ****
--- 1957,1963 ----
    if (! EQ (current_buffer->undo_list, Qt))
      record_delete (from, deletion);
    MODIFF++;
+   CHARS_MODIFF++;

    /* Relocate point as if it were a marker.  */
    if (from < PT)

*** imenu.el	Mon May  1 10:08:52 2006
--- imenu.el	Fri Jul 21 16:30:38 2006
***************
*** 968,982 ****
  (defvar imenu-buffer-menubar nil)

  (defvar imenu-menubar-modified-tick 0
!   "The value of (buffer-modified-tick) as of last call to `imenu-update-menubar'.")
  (make-variable-buffer-local 'imenu-menubar-modified-tick)

  (defun imenu-update-menubar ()
    (when (and (current-local-map)
  	     (keymapp (lookup-key (current-local-map) [menu-bar index]))
! 	     (not (eq (buffer-modified-tick)
  		      imenu-menubar-modified-tick)))
!     (setq imenu-menubar-modified-tick (buffer-modified-tick))
      (let ((index-alist (imenu--make-index-alist t)))
        ;; Don't bother updating if the index-alist has not changed
        ;; since the last time we did it.
--- 968,982 ----
  (defvar imenu-buffer-menubar nil)

  (defvar imenu-menubar-modified-tick 0
!   "The value of (buffer-chars-modified-tick) as of last call to `imenu-update-menubar'.")
  (make-variable-buffer-local 'imenu-menubar-modified-tick)

  (defun imenu-update-menubar ()
    (when (and (current-local-map)
  	     (keymapp (lookup-key (current-local-map) [menu-bar index]))
! 	     (not (eq (buffer-chars-modified-tick)
  		      imenu-menubar-modified-tick)))
!     (setq imenu-menubar-modified-tick (buffer-chars-modified-tick))
      (let ((index-alist (imenu--make-index-alist t)))
        ;; Don't bother updating if the index-alist has not changed
        ;; since the last time we did it.


[-- Attachment #3: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

Re: jit lock sit-for provokes redisplay provokes imenu

[Richard Stallman]

    1. Should the value returned by `buffer-chars-modified-tick' always
    (invariantly) imply the value returned by `buffer-modified-tick'?

I do not understand "imply" in this context.

								       With
    other words, suppose we have a thing called CHAR_MODIFF: Would we have
    to support the invariant

		      MODIFF >= CHAR_MODIFF >= SAVE_MODIFF

Yes, that is the idea.

		      if (MODIFF - 1 == SAVE_MODIFF)
			SAVE_MODIFF++;

    I think that's innocuous but, if someone wanted to increment SAVE_MODIFF
    in order to "ignore" a _text-property_ change only, the invariant above
    could get violated.  Is anyone aware of whether such a hack exists or is
    planned somewhere?

They should update CHAR_MODIFF too.

    3. `first-change-hook' currently runs whenever a text-property is
    assigned (by font-lock, for example).  I believe this should change with
    the new function too?

No, I think that should stay the same.  This change is safe because
ALL it will do is create CHAR_MODIFF and provide a function to examine
it.

Re: jit lock sit-for provokes redisplay provokes imenu

[Richard Stallman]

Rather than increment CHARS_MODIFF separately,
it should copy the value from MODIFF.
That way, values of CHARS_MODIFF will be comparable with values
that were copied from MODIFF.

Also, instead of setting CHARS_MODIFF explicitly in each place that
modifies the buffer, modify_region could set it from MODIFF, and
changing text properties could save and restore it to prevent it from
being incremented.  That means changes in fewer places, and no need
for each place that modifies the buffer to know about CHARS_MODIFF.

Re: jit lock sit-for provokes redisplay provokes imenu

[martin rudalics]

 >     1. Should the value returned by `buffer-chars-modified-tick' always
 >     (invariantly) imply the value returned by `buffer-modified-tick'?
 >
 > I do not understand "imply" in this context.

Sorry.  I was thinking about a function `buffer-chars-modified-p' and
intended to ask the following question:

1. Should the value returned by `buffer-chars-modified-p' always
    (invariantly) imply the value returned by `buffer-modified-p'?

Anyway, it's implied by the following.

 >
 > 								       With
 >     other words, suppose we have a thing called CHAR_MODIFF: Would we have
 >     to support the invariant
 >
 > 		      MODIFF >= CHAR_MODIFF >= SAVE_MODIFF
 >
 > Yes, that is the idea.

But now I'm in doubt whether I reasoned correctly.  It would mean that
when I save a buffer and thusly update save_modiff I'd have to update
char_modiff as well to preserve that invariant.  As a consequence,
clients like imenu would conclude that an insertion/deletion occurred
although, in fact, the buffer was only saved.

Re: jit lock sit-for provokes redisplay provokes imenu

[martin rudalics]

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

 > Rather than increment CHARS_MODIFF separately,
 > it should copy the value from MODIFF.
 > That way, values of CHARS_MODIFF will be comparable with values
 > that were copied from MODIFF.

Yes, but it will deprive you of eventually introducing a variable
SAVE_CHARS_MODIFF and tell whether an insertion/deletion occurred
since the last save because CHARS_MODIFF > SAVE_CHARS_MODIFF.

 > Also, instead of setting CHARS_MODIFF explicitly in each place that
 > modifies the buffer, modify_region could set it from MODIFF, and
 > changing text properties could save and restore it to prevent it from
 > being incremented.  That means changes in fewer places, and no need
 > for each place that modifies the buffer to know about CHARS_MODIFF.

Perhaps the cleanest solution would be to equip modify_region with an
additional parameter, say preserve_chars_modiff, and do everything in
modify_region.

In the attached patch I tried to follow your suggestions.

[-- Attachment #2: modiff.patch --]
[-- Type: text/plain, Size: 10041 bytes --]

*** buffer.c	Tue Jun  6 19:20:40 2006
--- buffer.c	Fri Jul 21 16:00:22 2006
***************
*** 371,376 ****
--- 371,377 ----
    BUF_ZV_BYTE (b) = BEG_BYTE;
    BUF_Z_BYTE (b) = BEG_BYTE;
    BUF_MODIFF (b) = 1;
+   BUF_CHARS_MODIFF (b) = 1;
    BUF_OVERLAY_MODIFF (b) = 1;
    BUF_SAVE_MODIFF (b) = 1;
    BUF_INTERVALS (b) = 0;
***************
*** 1145,1150 ****
--- 1146,1172 ----

    return make_number (BUF_MODIFF (buf));
  }
+ 
+ DEFUN ("buffer-chars-modified-tick", Fbuffer_chars_modified_tick, Sbuffer_chars_modified_tick,
+        0, 1, 0,
+        doc: /* Return BUFFER's tick counter, incremented for each insert/delete.
+ Each buffer has a tick counter which is incremented each time text in
+ that buffer is inserted or deleted.  It wraps around occasionally.
+ No argument or nil as argument means use current buffer as BUFFER.  */)
+      (buffer)
+      register Lisp_Object buffer;
+ {
+   register struct buffer *buf;
+   if (NILP (buffer))
+     buf = current_buffer;
+   else
+     {
+       CHECK_BUFFER (buffer);
+       buf = XBUFFER (buffer);
+     }
+ 
+   return make_number (BUF_CHARS_MODIFF (buf));
+ }
  \f
  DEFUN ("rename-buffer", Frename_buffer, Srename_buffer, 1, 2,
         "sRename buffer (to new name): \nP",
***************
*** 6014,6019 ****
--- 6036,6042 ----
    defsubr (&Sbuffer_modified_p);
    defsubr (&Sset_buffer_modified_p);
    defsubr (&Sbuffer_modified_tick);
+   defsubr (&Sbuffer_chars_modified_tick);
    defsubr (&Srename_buffer);
    defsubr (&Sother_buffer);
    defsubr (&Sbuffer_enable_undo);

*** buffer.h	Tue Apr 11 16:24:24 2006
--- buffer.h	Fri Jul 21 15:31:34 2006
***************
*** 82,87 ****
--- 82,90 ----
  /* Modification count.  */
  #define MODIFF (current_buffer->text->modiff)

+ /* Character modification count.  */
+ #define CHARS_MODIFF (current_buffer->text->chars_modiff)
+ 
  /* Overlay modification count.  */
  #define OVERLAY_MODIFF (current_buffer->text->overlay_modiff)

***************
*** 147,152 ****
--- 150,158 ----
  /* Modification count.  */
  #define BUF_MODIFF(buf) ((buf)->text->modiff)

+ /* Character modification count.  */
+ #define BUF_CHARS_MODIFF(buf) ((buf)->text->chars_modiff)
+ 
  /* Modification count as of last visit or save.  */
  #define BUF_SAVE_MODIFF(buf) ((buf)->text->save_modiff)

***************
*** 406,411 ****
--- 412,421 ----
  				   for this buffer.  It is incremented for
  				   each such event, and never otherwise
  				   changed.  */
+     int chars_modiff;		/* This counts insert/delete events
+ 				   for this buffer.  It is incremented for
+ 				   each such event, and never otherwise
+ 				   changed.  */
      int save_modiff;		/* Previous value of modiff, as of last
  				   time buffer visited or saved a file.  */


*** insdel.c	Tue Apr 11 16:24:24 2006
--- insdel.c	Sat Jul 22 09:48:24 2006
***************
*** 1007,1012 ****
--- 1007,1013 ----
       will add up to the right stuff in the undo list.  */
    record_insert (PT, nchars);
    MODIFF++;
+   CHARS_MODIFF = MODIFF;

    bcopy (string, GPT_ADDR, nbytes);

***************
*** 1144,1149 ****
--- 1145,1151 ----

    record_insert (PT, nchars);
    MODIFF++;
+   CHARS_MODIFF = MODIFF;

    GAP_SIZE -= outgoing_nbytes;
    GPT += nchars;
***************
*** 1295,1300 ****
--- 1297,1303 ----

    record_insert (PT, nchars);
    MODIFF++;
+   CHARS_MODIFF = MODIFF;

    GAP_SIZE -= outgoing_nbytes;
    GPT += nchars;
***************
*** 1403,1408 ****
--- 1406,1412 ----
    if (len == 0)
      evaporate_overlays (from);
    MODIFF++;
+   CHARS_MODIFF = MODIFF;
  }

  /* Like adjust_after_replace, but doesn't require PREV_TEXT.
***************
*** 1453,1458 ****
--- 1457,1463 ----
    if (len == 0)
      evaporate_overlays (from);
    MODIFF++;
+   CHARS_MODIFF = MODIFF;
  }

  /* Record undo information, adjust markers and position keepers for an
***************
*** 1645,1650 ****
--- 1650,1656 ----
    CHECK_MARKERS ();

    MODIFF++;
+   CHARS_MODIFF = MODIFF;
    UNGCPRO;

    signal_after_change (from, nchars_del, GPT - from);
***************
*** 1769,1774 ****
--- 1775,1781 ----
    CHECK_MARKERS ();

    MODIFF++;
+   CHARS_MODIFF = MODIFF;
  }
  \f
  /* Delete characters in current buffer
***************
*** 1950,1955 ****
--- 1957,1963 ----
    if (! EQ (current_buffer->undo_list, Qt))
      record_delete (from, deletion);
    MODIFF++;
+   CHARS_MODIFF = MODIFF;

    /* Relocate point as if it were a marker.  */
    if (from < PT)
***************
*** 2009,2014 ****
--- 2017,2023 ----
    if (MODIFF <= SAVE_MODIFF)
      record_first_change ();
    MODIFF++;
+   CHARS_MODIFF = MODIFF;

    buffer->point_before_scroll = Qnil;


*** textprop.c	Sun Jul  2 09:50:00 2006
--- textprop.c	Sat Jul 22 10:44:34 2006
***************
*** 1247,1253 ****
      }

    if (BUFFERP (object))
!     modify_region (XBUFFER (object), XINT (start), XINT (end));

    /* We are at the beginning of interval I, with LEN chars to scan.  */
    for (;;)
--- 1247,1257 ----
      }

    if (BUFFERP (object))
!     {
!       int save_chars_modiff = CHARS_MODIFF;
!       modify_region (XBUFFER (object), XINT (start), XINT (end));
!       CHARS_MODIFF = save_chars_modiff;
!     }

    /* We are at the beginning of interval I, with LEN chars to scan.  */
    for (;;)
***************
*** 1387,1393 ****
      }

    if (BUFFERP (object))
!     modify_region (XBUFFER (object), XINT (start), XINT (end));

    set_text_properties_1 (start, end, properties, object, i);

--- 1391,1401 ----
      }

    if (BUFFERP (object))
!     {
!       int save_chars_modiff = CHARS_MODIFF;
!       modify_region (XBUFFER (object), XINT (start), XINT (end));
!       CHARS_MODIFF = save_chars_modiff;
!     }

    set_text_properties_1 (start, end, properties, object, i);

***************
*** 1535,1541 ****
      }

    if (BUFFERP (object))
!     modify_region (XBUFFER (object), XINT (start), XINT (end));

    /* We are at the beginning of an interval, with len to scan */
    for (;;)
--- 1543,1553 ----
      }

    if (BUFFERP (object))
!     {
!       int save_chars_modiff = CHARS_MODIFF;
!       modify_region (XBUFFER (object), XINT (start), XINT (end));
!       CHARS_MODIFF = save_chars_modiff;
!     }

    /* We are at the beginning of an interval, with len to scan */
    for (;;)
***************
*** 1649,1655 ****
  	  if (LENGTH (i) == len)
  	    {
  	      if (!modified && BUFFERP (object))
! 		modify_region (XBUFFER (object), XINT (start), XINT (end));
  	      remove_properties (Qnil, properties, i, object);
  	      if (BUFFERP (object))
  		signal_after_change (XINT (start), XINT (end) - XINT (start),
--- 1661,1671 ----
  	  if (LENGTH (i) == len)
  	    {
  	      if (!modified && BUFFERP (object))
! 		{
! 		  int save_chars_modiff = CHARS_MODIFF;
! 		  modify_region (XBUFFER (object), XINT (start), XINT (end));
! 		  CHARS_MODIFF = save_chars_modiff;
! 		}
  	      remove_properties (Qnil, properties, i, object);
  	      if (BUFFERP (object))
  		signal_after_change (XINT (start), XINT (end) - XINT (start),
***************
*** 1662,1668 ****
  	  i = split_interval_left (i, len);
  	  copy_properties (unchanged, i);
  	  if (!modified && BUFFERP (object))
! 	    modify_region (XBUFFER (object), XINT (start), XINT (end));
  	  remove_properties (Qnil, properties, i, object);
  	  if (BUFFERP (object))
  	    signal_after_change (XINT (start), XINT (end) - XINT (start),
--- 1678,1688 ----
  	  i = split_interval_left (i, len);
  	  copy_properties (unchanged, i);
  	  if (!modified && BUFFERP (object))
! 	    {
! 	      int save_chars_modiff = CHARS_MODIFF;
! 	      modify_region (XBUFFER (object), XINT (start), XINT (end));
! 	      CHARS_MODIFF = save_chars_modiff;
! 	    }
  	  remove_properties (Qnil, properties, i, object);
  	  if (BUFFERP (object))
  	    signal_after_change (XINT (start), XINT (end) - XINT (start),
***************
*** 1673,1679 ****
        if (interval_has_some_properties_list (properties, i))
  	{
  	  if (!modified && BUFFERP (object))
! 	    modify_region (XBUFFER (object), XINT (start), XINT (end));
  	  remove_properties (Qnil, properties, i, object);
  	  modified = 1;
  	}
--- 1693,1703 ----
        if (interval_has_some_properties_list (properties, i))
  	{
  	  if (!modified && BUFFERP (object))
! 	    {
! 	      int save_chars_modiff = CHARS_MODIFF;
! 	      modify_region (XBUFFER (object), XINT (start), XINT (end));
! 	      CHARS_MODIFF = save_chars_modiff;
! 	    }
  	  remove_properties (Qnil, properties, i, object);
  	  modified = 1;
  	}

*** imenu.el	Mon May  1 10:08:52 2006
--- imenu.el	Fri Jul 21 16:30:38 2006
***************
*** 968,982 ****
  (defvar imenu-buffer-menubar nil)

  (defvar imenu-menubar-modified-tick 0
!   "The value of (buffer-modified-tick) as of last call to `imenu-update-menubar'.")
  (make-variable-buffer-local 'imenu-menubar-modified-tick)

  (defun imenu-update-menubar ()
    (when (and (current-local-map)
  	     (keymapp (lookup-key (current-local-map) [menu-bar index]))
! 	     (not (eq (buffer-modified-tick)
  		      imenu-menubar-modified-tick)))
!     (setq imenu-menubar-modified-tick (buffer-modified-tick))
      (let ((index-alist (imenu--make-index-alist t)))
        ;; Don't bother updating if the index-alist has not changed
        ;; since the last time we did it.
--- 968,982 ----
  (defvar imenu-buffer-menubar nil)

  (defvar imenu-menubar-modified-tick 0
!   "The value of (buffer-chars-modified-tick) as of last call to `imenu-update-menubar'.")
  (make-variable-buffer-local 'imenu-menubar-modified-tick)

  (defun imenu-update-menubar ()
    (when (and (current-local-map)
  	     (keymapp (lookup-key (current-local-map) [menu-bar index]))
! 	     (not (eq (buffer-chars-modified-tick)
  		      imenu-menubar-modified-tick)))
!     (setq imenu-menubar-modified-tick (buffer-chars-modified-tick))
      (let ((index-alist (imenu--make-index-alist t)))
        ;; Don't bother updating if the index-alist has not changed
        ;; since the last time we did it.


[-- Attachment #3: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

Re: jit lock sit-for provokes redisplay provokes imenu

[martin rudalics]

 >  > Rather than increment CHARS_MODIFF separately,
 >  > it should copy the value from MODIFF.
 >  > That way, values of CHARS_MODIFF will be comparable with values
 >  > that were copied from MODIFF.
 >
 > Yes, but it will deprive you of eventually introducing a variable
 > SAVE_CHARS_MODIFF and tell whether an insertion/deletion occurred
 > since the last save because CHARS_MODIFF > SAVE_CHARS_MODIFF.

Rereading this I think that there should not be any problem.
There's also no reasonable use of such a variable I can think of.
Hence, please ignore this part of the message.

Re: jit lock sit-for provokes redisplay provokes imenu

[Richard Stallman]

     >     other words, suppose we have a thing called CHAR_MODIFF: Would we have
     >     to support the invariant
     >
     > 		      MODIFF >= CHAR_MODIFF >= SAVE_MODIFF
     >
     > Yes, that is the idea.

    But now I'm in doubt whether I reasoned correctly.  It would mean that
    when I save a buffer and thusly update save_modiff I'd have to update
    char_modiff as well to preserve that invariant.

That is true.

We could drop the requirement thaht CHAR_MODIFF be >= SAVE_MODIFF.
That would be fine, for imenu.  Is there another use which
needs to compare those two values?

Re: jit lock sit-for provokes redisplay provokes imenu

[Richard Stallman]

     > Rather than increment CHARS_MODIFF separately,
     > it should copy the value from MODIFF.
     > That way, values of CHARS_MODIFF will be comparable with values
     > that were copied from MODIFF.

    Yes, but it will deprive you of eventually introducing a variable
    SAVE_CHARS_MODIFF and tell whether an insertion/deletion occurred
    since the last save because CHARS_MODIFF > SAVE_CHARS_MODIFF.

I do not see how you reach that conclusion.  As far as I can see, if
we want to someday have SAVE_CHARS_MODIFF, my way of handling
CHARS_MODIFF will not be any obstacle to doing so.

However, with my way of handling CHARS_MODIFF, there is no need
for a separate SAVE_CHARS_MODIFF.  SAVE_MODIFF can be used.
If CHARS_MODIFF > SAVE_MODIFF, then the text in the buffer
has been altered.  If SAVE_MODIFF >= CHARS_MODIFF, then the
text has not been altered since the last save.

This is assuming that saving copies MODIFF to SAVE_MODIFF,
and that we copy MODIFF to CHARS_MODIFF whenever we change
the characters in the buffer.

    +     int chars_modiff;		/* This counts insert/delete events
    + 				   for this buffer.  It is incremented for
    + 				   each such event, and never otherwise
    + 				   changed.  */

That comment is not true for the current code.

Aside from that, your latest patch is clean enough.

    Perhaps the cleanest solution would be to equip modify_region with an
    additional parameter, say preserve_chars_modiff, and do everything in
    modify_region.

It might be better.  Please give it a try and see.

Re: jit lock sit-for provokes redisplay provokes imenu

[martin rudalics]

>     But now I'm in doubt whether I reasoned correctly.  It would mean that
>     when I save a buffer and thusly update save_modiff I'd have to update
>     char_modiff as well to preserve that invariant.
> 
> That is true.
> 
> We could drop the requirement thaht CHAR_MODIFF be >= SAVE_MODIFF.
> That would be fine, for imenu.  Is there another use which
> needs to compare those two values?

Probably.  But that's no reason to update CHAR_MODIFF when a buffer is
saved.  You already explained everything in the other subthread.

Re: jit lock sit-for provokes redisplay provokes imenu

[martin rudalics]

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

 > However, with my way of handling CHARS_MODIFF, there is no need
 > for a separate SAVE_CHARS_MODIFF.  SAVE_MODIFF can be used.
 > If CHARS_MODIFF > SAVE_MODIFF, then the text in the buffer
 > has been altered.  If SAVE_MODIFF >= CHARS_MODIFF, then the
 > text has not been altered since the last save.

Yes.  If needed we can always add:

DEFUN ("buffer-chars-modified-p", Fbuffer_chars_modified_p, Sbuffer_chars_modified_p,
        0, 1, 0,
        doc: /* Return t if a character changed in BUFFER since its file was last read or saved.
No argument or nil as argument means use current buffer as BUFFER.  */)
      (buffer)
      register Lisp_Object buffer;
{
   register struct buffer *buf;
   if (NILP (buffer))
     buf = current_buffer;
   else
     {
       CHECK_BUFFER (buffer);
       buf = XBUFFER (buffer);
     }

   return BUF_SAVE_MODIFF (buf) < BUF_CHARS_MODIFF (buf) ? Qt : Qnil;
}

 >     +     int chars_modiff;		/* This counts insert/delete events
 >     + 				   for this buffer.  It is incremented for
 >     + 				   each such event, and never otherwise
 >     + 				   changed.  */
 >
 > That comment is not true for the current code.
 >
 >     Perhaps the cleanest solution would be to equip modify_region with an
 >     additional parameter, say preserve_chars_modiff, and do everything in
 >     modify_region.
 >
 > It might be better.  Please give it a try and see.

Please consult the attached patch.

Simon, please try with your settings whether the patch makes those extra
calls to `imenu-update-menubar' disappear.  For some strange reason, I
was not able to reproduce your behavior so far.

[-- Attachment #2: chars-modiff.patch --]
[-- Type: text/plain, Size: 15516 bytes --]

*** buffer.c	Tue Jun  6 19:20:40 2006
--- buffer.c	Wed Jul 26 12:30:46 2006
***************
*** 371,376 ****
--- 371,377 ----
    BUF_ZV_BYTE (b) = BEG_BYTE;
    BUF_Z_BYTE (b) = BEG_BYTE;
    BUF_MODIFF (b) = 1;
+   BUF_CHARS_MODIFF (b) = 1;
    BUF_OVERLAY_MODIFF (b) = 1;
    BUF_SAVE_MODIFF (b) = 1;
    BUF_INTERVALS (b) = 0;
***************
*** 1145,1150 ****
--- 1146,1176 ----

    return make_number (BUF_MODIFF (buf));
  }
+ 
+ DEFUN ("buffer-chars-modified-tick", Fbuffer_chars_modified_tick, Sbuffer_chars_modified_tick,
+        0, 1, 0,
+        doc: /* Return BUFFER's character-change tick counter.
+ Each buffer has a character-change tick counter, which is set to the
+ value of the buffer's tick counter \(see `buffer-modified-tick'), each
+ time text in that buffer is inserted or deleted.  By comparing the
+ values returned by two individual calls of buffer-chars-modified-tick,
+ you can tell whether a character change occurred in that buffer in
+ between these calls.  No argument or nil as argument means use current
+ buffer as BUFFER.  */)
+      (buffer)
+      register Lisp_Object buffer;
+ {
+   register struct buffer *buf;
+   if (NILP (buffer))
+     buf = current_buffer;
+   else
+     {
+       CHECK_BUFFER (buffer);
+       buf = XBUFFER (buffer);
+     }
+ 
+   return make_number (BUF_CHARS_MODIFF (buf));
+ }
  \f
  DEFUN ("rename-buffer", Frename_buffer, Srename_buffer, 1, 2,
         "sRename buffer (to new name): \nP",
***************
*** 6014,6019 ****
--- 6040,6046 ----
    defsubr (&Sbuffer_modified_p);
    defsubr (&Sset_buffer_modified_p);
    defsubr (&Sbuffer_modified_tick);
+   defsubr (&Sbuffer_chars_modified_tick);
    defsubr (&Srename_buffer);
    defsubr (&Sother_buffer);
    defsubr (&Sbuffer_enable_undo);

*** buffer.h	Tue Apr 11 16:24:24 2006
--- buffer.h	Wed Jul 26 11:08:36 2006
***************
*** 82,87 ****
--- 82,90 ----
  /* Modification count.  */
  #define MODIFF (current_buffer->text->modiff)

+ /* Character modification count.  */
+ #define CHARS_MODIFF (current_buffer->text->chars_modiff)
+ 
  /* Overlay modification count.  */
  #define OVERLAY_MODIFF (current_buffer->text->overlay_modiff)

***************
*** 147,152 ****
--- 150,158 ----
  /* Modification count.  */
  #define BUF_MODIFF(buf) ((buf)->text->modiff)

+ /* Character modification count.  */
+ #define BUF_CHARS_MODIFF(buf) ((buf)->text->chars_modiff)
+ 
  /* Modification count as of last visit or save.  */
  #define BUF_SAVE_MODIFF(buf) ((buf)->text->save_modiff)

***************
*** 406,411 ****
--- 412,421 ----
  				   for this buffer.  It is incremented for
  				   each such event, and never otherwise
  				   changed.  */
+     int chars_modiff;		/* This is modified with character-change
+ 				   events for this buffer.  It is set to
+ 				   modiff for each such event, and never
+ 				   otherwise changed.  */
      int save_modiff;		/* Previous value of modiff, as of last
  				   time buffer visited or saved a file.  */


*** casefiddle.c	Tue Apr 11 16:24:24 2006
--- casefiddle.c	Tue Jul 25 18:41:12 2006
***************
*** 189,195 ****
    validate_region (&b, &e);
    start = XFASTINT (b);
    end = XFASTINT (e);
!   modify_region (current_buffer, start, end);
    record_change (start, end - start);
    start_byte = CHAR_TO_BYTE (start);
    end_byte = CHAR_TO_BYTE (end);
--- 189,195 ----
    validate_region (&b, &e);
    start = XFASTINT (b);
    end = XFASTINT (e);
!   modify_region (current_buffer, start, end, 0);
    record_change (start, end - start);
    start_byte = CHAR_TO_BYTE (start);
    end_byte = CHAR_TO_BYTE (end);

*** editfns.c	Sun Jul  2 09:50:00 2006
--- editfns.c	Tue Jul 25 18:41:54 2006
***************
*** 2785,2791 ****
  	  if (! changed)
  	    {
  	      changed = pos;
! 	      modify_region (current_buffer, changed, XINT (end));

  	      if (! NILP (noundo))
  		{
--- 2785,2791 ----
  	  if (! changed)
  	    {
  	      changed = pos;
! 	      modify_region (current_buffer, changed, XINT (end), 0);

  	      if (! NILP (noundo))
  		{
***************
*** 2897,2903 ****
    pos = XINT (start);
    pos_byte = CHAR_TO_BYTE (pos);
    end_pos = XINT (end);
!   modify_region (current_buffer, pos, XINT (end));

    cnt = 0;
    for (; pos < end_pos; )
--- 2897,2903 ----
    pos = XINT (start);
    pos_byte = CHAR_TO_BYTE (pos);
    end_pos = XINT (end);
!   modify_region (current_buffer, pos, XINT (end), 0);

    cnt = 0;
    for (; pos < end_pos; )
***************
*** 4162,4168 ****

    if (end1 == start2)		/* adjacent regions */
      {
!       modify_region (current_buffer, start1, end2);
        record_change (start1, len1 + len2);

        tmp_interval1 = copy_intervals (cur_intv, start1, len1);
--- 4162,4168 ----

    if (end1 == start2)		/* adjacent regions */
      {
!       modify_region (current_buffer, start1, end2, 0);
        record_change (start1, len1 + len2);

        tmp_interval1 = copy_intervals (cur_intv, start1, len1);
***************
*** 4218,4225 ****
          {
  	  USE_SAFE_ALLOCA;

!           modify_region (current_buffer, start1, end1);
!           modify_region (current_buffer, start2, end2);
            record_change (start1, len1);
            record_change (start2, len2);
            tmp_interval1 = copy_intervals (cur_intv, start1, len1);
--- 4218,4225 ----
          {
  	  USE_SAFE_ALLOCA;

!           modify_region (current_buffer, start1, end1, 0);
!           modify_region (current_buffer, start2, end2, 0);
            record_change (start1, len1);
            record_change (start2, len2);
            tmp_interval1 = copy_intervals (cur_intv, start1, len1);
***************
*** 4248,4254 ****
          {
  	  USE_SAFE_ALLOCA;

!           modify_region (current_buffer, start1, end2);
            record_change (start1, (end2 - start1));
            tmp_interval1 = copy_intervals (cur_intv, start1, len1);
            tmp_interval_mid = copy_intervals (cur_intv, end1, len_mid);
--- 4248,4254 ----
          {
  	  USE_SAFE_ALLOCA;

!           modify_region (current_buffer, start1, end2, 0);
            record_change (start1, (end2 - start1));
            tmp_interval1 = copy_intervals (cur_intv, start1, len1);
            tmp_interval_mid = copy_intervals (cur_intv, end1, len_mid);
***************
*** 4279,4285 ****
  	  USE_SAFE_ALLOCA;

            record_change (start1, (end2 - start1));
!           modify_region (current_buffer, start1, end2);

            tmp_interval1 = copy_intervals (cur_intv, start1, len1);
            tmp_interval_mid = copy_intervals (cur_intv, end1, len_mid);
--- 4279,4285 ----
  	  USE_SAFE_ALLOCA;

            record_change (start1, (end2 - start1));
!           modify_region (current_buffer, start1, end2, 0);

            tmp_interval1 = copy_intervals (cur_intv, start1, len1);
            tmp_interval_mid = copy_intervals (cur_intv, end1, len_mid);

*** insdel.c	Tue Apr 11 16:24:24 2006
--- insdel.c	Tue Jul 25 18:42:26 2006
***************
*** 1007,1012 ****
--- 1007,1013 ----
       will add up to the right stuff in the undo list.  */
    record_insert (PT, nchars);
    MODIFF++;
+   CHARS_MODIFF = MODIFF;

    bcopy (string, GPT_ADDR, nbytes);

***************
*** 1144,1149 ****
--- 1145,1151 ----

    record_insert (PT, nchars);
    MODIFF++;
+   CHARS_MODIFF = MODIFF;

    GAP_SIZE -= outgoing_nbytes;
    GPT += nchars;
***************
*** 1295,1300 ****
--- 1297,1303 ----

    record_insert (PT, nchars);
    MODIFF++;
+   CHARS_MODIFF = MODIFF;

    GAP_SIZE -= outgoing_nbytes;
    GPT += nchars;
***************
*** 1403,1408 ****
--- 1406,1412 ----
    if (len == 0)
      evaporate_overlays (from);
    MODIFF++;
+   CHARS_MODIFF = MODIFF;
  }

  /* Like adjust_after_replace, but doesn't require PREV_TEXT.
***************
*** 1453,1458 ****
--- 1457,1463 ----
    if (len == 0)
      evaporate_overlays (from);
    MODIFF++;
+   CHARS_MODIFF = MODIFF;
  }

  /* Record undo information, adjust markers and position keepers for an
***************
*** 1645,1650 ****
--- 1650,1656 ----
    CHECK_MARKERS ();

    MODIFF++;
+   CHARS_MODIFF = MODIFF;
    UNGCPRO;

    signal_after_change (from, nchars_del, GPT - from);
***************
*** 1769,1774 ****
--- 1775,1781 ----
    CHECK_MARKERS ();

    MODIFF++;
+   CHARS_MODIFF = MODIFF;
  }
  \f
  /* Delete characters in current buffer
***************
*** 1950,1955 ****
--- 1957,1963 ----
    if (! EQ (current_buffer->undo_list, Qt))
      record_delete (from, deletion);
    MODIFF++;
+   CHARS_MODIFF = MODIFF;

    /* Relocate point as if it were a marker.  */
    if (from < PT)
***************
*** 1990,2001 ****
     character positions START to END.  This checks the read-only
     properties of the region, calls the necessary modification hooks,
     and warns the next redisplay that it should pay attention to that
!    area.  */

! void
! modify_region (buffer, start, end)
       struct buffer *buffer;
!      int start, end;
  {
    struct buffer *old_buffer = current_buffer;

--- 1998,2012 ----
     character positions START to END.  This checks the read-only
     properties of the region, calls the necessary modification hooks,
     and warns the next redisplay that it should pay attention to that
!    area.

!    If PRESERVE_CHARS_MODIFF is non-zero, do not update CHARS_MODIFF.
!    Otherwise set CHARS_MODIFF to the new value of MODIFF.  */
! 
! VOID
! modify_region (buffer, start, end, preserve_chars_modiff)
       struct buffer *buffer;
!      int start, end, preserve_chars_modiff;
  {
    struct buffer *old_buffer = current_buffer;

***************
*** 2009,2014 ****
--- 2020,2027 ----
    if (MODIFF <= SAVE_MODIFF)
      record_first_change ();
    MODIFF++;
+   if (! preserve_chars_modiff)
+     CHARS_MODIFF = MODIFF;

    buffer->point_before_scroll = Qnil;


*** lisp.h	Tue Jun  6 19:20:40 2006
--- lisp.h	Tue Jul 25 18:42:54 2006
***************
*** 2451,2457 ****
  extern void del_range_byte P_ ((int, int, int));
  extern void del_range_both P_ ((int, int, int, int, int));
  extern Lisp_Object del_range_2 P_ ((int, int, int, int, int));
! extern void modify_region P_ ((struct buffer *, int, int));
  extern void prepare_to_modify_buffer P_ ((int, int, int *));
  extern void signal_before_change P_ ((int, int, int *));
  extern void signal_after_change P_ ((int, int, int));
--- 2451,2457 ----
  extern void del_range_byte P_ ((int, int, int));
  extern void del_range_both P_ ((int, int, int, int, int));
  extern Lisp_Object del_range_2 P_ ((int, int, int, int, int));
! extern void modify_region P_ ((struct buffer *, int, int, int));
  extern void prepare_to_modify_buffer P_ ((int, int, int *));
  extern void signal_before_change P_ ((int, int, int *));
  extern void signal_after_change P_ ((int, int, int));

*** textprop.c	Sun Jul  2 09:50:00 2006
--- textprop.c	Tue Jul 25 18:45:36 2006
***************
*** 1247,1253 ****
      }

    if (BUFFERP (object))
!     modify_region (XBUFFER (object), XINT (start), XINT (end));

    /* We are at the beginning of interval I, with LEN chars to scan.  */
    for (;;)
--- 1247,1253 ----
      }

    if (BUFFERP (object))
!     modify_region (XBUFFER (object), XINT (start), XINT (end), 1);

    /* We are at the beginning of interval I, with LEN chars to scan.  */
    for (;;)
***************
*** 1387,1393 ****
      }

    if (BUFFERP (object))
!     modify_region (XBUFFER (object), XINT (start), XINT (end));

    set_text_properties_1 (start, end, properties, object, i);

--- 1387,1393 ----
      }

    if (BUFFERP (object))
!     modify_region (XBUFFER (object), XINT (start), XINT (end), 1);

    set_text_properties_1 (start, end, properties, object, i);

***************
*** 1535,1541 ****
      }

    if (BUFFERP (object))
!     modify_region (XBUFFER (object), XINT (start), XINT (end));

    /* We are at the beginning of an interval, with len to scan */
    for (;;)
--- 1535,1541 ----
      }

    if (BUFFERP (object))
!     modify_region (XBUFFER (object), XINT (start), XINT (end), 1);

    /* We are at the beginning of an interval, with len to scan */
    for (;;)
***************
*** 1649,1655 ****
  	  if (LENGTH (i) == len)
  	    {
  	      if (!modified && BUFFERP (object))
! 		modify_region (XBUFFER (object), XINT (start), XINT (end));
  	      remove_properties (Qnil, properties, i, object);
  	      if (BUFFERP (object))
  		signal_after_change (XINT (start), XINT (end) - XINT (start),
--- 1649,1655 ----
  	  if (LENGTH (i) == len)
  	    {
  	      if (!modified && BUFFERP (object))
! 		modify_region (XBUFFER (object), XINT (start), XINT (end), 1);
  	      remove_properties (Qnil, properties, i, object);
  	      if (BUFFERP (object))
  		signal_after_change (XINT (start), XINT (end) - XINT (start),
***************
*** 1662,1668 ****
  	  i = split_interval_left (i, len);
  	  copy_properties (unchanged, i);
  	  if (!modified && BUFFERP (object))
! 	    modify_region (XBUFFER (object), XINT (start), XINT (end));
  	  remove_properties (Qnil, properties, i, object);
  	  if (BUFFERP (object))
  	    signal_after_change (XINT (start), XINT (end) - XINT (start),
--- 1662,1668 ----
  	  i = split_interval_left (i, len);
  	  copy_properties (unchanged, i);
  	  if (!modified && BUFFERP (object))
! 	    modify_region (XBUFFER (object), XINT (start), XINT (end), 1);
  	  remove_properties (Qnil, properties, i, object);
  	  if (BUFFERP (object))
  	    signal_after_change (XINT (start), XINT (end) - XINT (start),
***************
*** 1673,1679 ****
        if (interval_has_some_properties_list (properties, i))
  	{
  	  if (!modified && BUFFERP (object))
! 	    modify_region (XBUFFER (object), XINT (start), XINT (end));
  	  remove_properties (Qnil, properties, i, object);
  	  modified = 1;
  	}
--- 1673,1679 ----
        if (interval_has_some_properties_list (properties, i))
  	{
  	  if (!modified && BUFFERP (object))
! 	    modify_region (XBUFFER (object), XINT (start), XINT (end), 1);
  	  remove_properties (Qnil, properties, i, object);
  	  modified = 1;
  	}

*** imenu.el	Mon May  1 10:08:52 2006
--- imenu.el	Wed Jul 26 11:20:28 2006
***************
*** 968,982 ****
  (defvar imenu-buffer-menubar nil)

  (defvar imenu-menubar-modified-tick 0
!   "The value of (buffer-modified-tick) as of last call to `imenu-update-menubar'.")
  (make-variable-buffer-local 'imenu-menubar-modified-tick)

  (defun imenu-update-menubar ()
    (when (and (current-local-map)
  	     (keymapp (lookup-key (current-local-map) [menu-bar index]))
! 	     (not (eq (buffer-modified-tick)
! 		      imenu-menubar-modified-tick)))
!     (setq imenu-menubar-modified-tick (buffer-modified-tick))
      (let ((index-alist (imenu--make-index-alist t)))
        ;; Don't bother updating if the index-alist has not changed
        ;; since the last time we did it.
--- 968,981 ----
  (defvar imenu-buffer-menubar nil)

  (defvar imenu-menubar-modified-tick 0
!   "The value of (buffer-chars-modified-tick) as of last call to `imenu-update-menubar'.")
  (make-variable-buffer-local 'imenu-menubar-modified-tick)

  (defun imenu-update-menubar ()
    (when (and (current-local-map)
  	     (keymapp (lookup-key (current-local-map) [menu-bar index]))
! 	     (/= (buffer-chars-modified-tick) imenu-menubar-modified-tick))
!     (setq imenu-menubar-modified-tick (buffer-chars-modified-tick))
      (let ((index-alist (imenu--make-index-alist t)))
        ;; Don't bother updating if the index-alist has not changed
        ;; since the last time we did it.


[-- Attachment #3: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

RE: jit lock sit-for provokes redisplay provokes imenu

[Marshall, Simon]

> Simon, please try with your settings whether the patch makes 
> those extra
> calls to `imenu-update-menubar' disappear.  For some strange reason, I
> was not able to reproduce your behavior so far.

Apart from the return type of VOID for modify_region(), your patch is fine
for me.

BTW, so is your mouse-autoselect-window patch of last week.

Thanks, Simon.

Re: jit lock sit-for provokes redisplay provokes imenu

[Stefan Monnier]

>   return BUF_SAVE_MODIFF (buf) < BUF_CHARS_MODIFF (buf) ? Qt : Qnil;

Isn't this vulnerable to problems when MODIFF wraps around?


        Stefan

Re: jit lock sit-for provokes redisplay provokes imenu

[martin rudalics]

> Apart from the return type of VOID for modify_region(), your patch is fine
> for me.

Thanks for noticing (and testing).

martin

Re: jit lock sit-for provokes redisplay provokes imenu

[martin rudalics]

>>  return BUF_SAVE_MODIFF (buf) < BUF_CHARS_MODIFF (buf) ? Qt : Qnil;
> 
> 
> Isn't this vulnerable to problems when MODIFF wraps around?

Hmmm...  Do you think it's more vulnerable than `buffer-modified-p' or
`update_menu_bar'?  I probably don't understand the latter correctly -
presumably they could try to do something special for the case
BUF_SAVE_MODIFF > BUF_MODIFF because that _would_ indicate a
wrap-around.  Apparently, there never was a problem but maybe I'm
completely wrong here.

Re: jit lock sit-for provokes redisplay provokes imenu

[Richard Stallman]

    >   return BUF_SAVE_MODIFF (buf) < BUF_CHARS_MODIFF (buf) ? Qt : Qnil;

    Isn't this vulnerable to problems when MODIFF wraps around?

If the buffer is modified 1000 times a second, with never a pause, it
could wrap around in 24 days.  If we make the field unsigned, that
would increase to 48 days.

However, the Lisp level representation of this (returned by
buffer-modified-tick) is 3 bits smaller, so it would wrap around in
just 6 days.

We could declare the fields long long at the C level, and then they
would never wrap around (assuming there are no compilers that barf on
long long.)  But we would still have the problem at the Lisp level.

It isn't clear how we could fix this.  Meanwhile,
in practice, I think the problem ever happens.
So I think we should deal with it after the release.

Re: jit lock sit-for provokes redisplay provokes imenu

[David Kastrup]

Richard Stallman <rms@gnu.org> writes:

>     >   return BUF_SAVE_MODIFF (buf) < BUF_CHARS_MODIFF (buf) ? Qt : Qnil;
>
>     Isn't this vulnerable to problems when MODIFF wraps around?
>
> If the buffer is modified 1000 times a second, with never a pause, it
> could wrap around in 24 days.  If we make the field unsigned, that
> would increase to 48 days.
>
> However, the Lisp level representation of this (returned by
> buffer-modified-tick) is 3 bits smaller, so it would wrap around in
> just 6 days.
>
> We could declare the fields long long at the C level, and then they
> would never wrap around (assuming there are no compilers that barf on
> long long.)  But we would still have the problem at the Lisp level.
>
> It isn't clear how we could fix this.  Meanwhile,
> in practice, I think the problem ever happens.
> So I think we should deal with it after the release.

Wouldn't it be safer if such counts would be compared for equality
only?  That way, a wrapping counter will cause a one-time problem only
once he completely catches up with a non-wrapped counter, instead of
causing a permanent problem _until_ he catches up.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: jit lock sit-for provokes redisplay provokes imenu

[Stefan Monnier]

>>> return BUF_SAVE_MODIFF (buf) < BUF_CHARS_MODIFF (buf) ? Qt : Qnil;
>> Isn't this vulnerable to problems when MODIFF wraps around?

> Hmmm...  Do you think it's more vulnerable than `buffer-modified-p' or
> `update_menu_bar'?

Hmm... good point.  I just assumed that buffer-modified-p used "!=" instead
of "<", but indeed it uses "<" which makes it vulnerable as well.  I guess
wraparounds are sufficiently unlikely, then.


        Stefan

Re: jit lock sit-for provokes redisplay provokes imenu

[Richard Stallman]

    Wouldn't it be safer if such counts would be compared for equality
    only?

It would be, but I doubt it is possible in general
to use only equality comparisons.  Does someone want to study
the question and see?

Re: jit lock sit-for provokes redisplay provokes imenu

[David Kastrup]

Richard Stallman <rms@gnu.org> writes:

>     Wouldn't it be safer if such counts would be compared for equality
>     only?
>
> It would be, but I doubt it is possible in general
> to use only equality comparisons.  Does someone want to study
> the question and see?

Another possibility would be to check the sign of their difference
instead of comparing them directly.

Then wraparound is not a problem, instead one of them has to have
moved more than half the total range away from the other before things
get ugly.

Probably less likely.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: jit lock sit-for provokes redisplay provokes imenu

[Richard Stallman]

    Another possibility would be to check the sign of their difference
    instead of comparing them directly.

    Then wraparound is not a problem, instead one of them has to have
    moved more than half the total range away from the other before things
    get ugly.

It might happen less often, but if people really edit a buffer enough
to make the count wrap around, they could do so without saving it,
and this change wouldn't help in such a case.

Re: jit lock sit-for provokes redisplay provokes imenu

[martin rudalics]

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

Attached find ChangeLog entries and patches for this.

martin.

[-- Attachment #2: modiff.patch --]
[-- Type: text/plain, Size: 16881 bytes --]

2006-08-03  Martin Rudalics  <rudalics@gmx.at>

	* buffer.h (struct buffer_text): New field chars_modiff.
	(CHARS_MODIFF, BUF_CHARS_MODIFF): New macros.
	* buffer.c (Fbuffer_chars_modified_tick): New function returning
	value of BUF_CHARS_MODIFF.
	(syms_of_buffer): Defsubr it.
	(Fget_buffer_create): Initialize BUF_CHARS_MODIFF.
	* INSDEL.c (modify_region): New argument preserve_chars_modiff.
	Set CHARS_MODIFF to MODIFF provided preserve_chars_modiff is zero.
	(insert_1_both, insert_from_string_1, insert_from_buffer_1)
	(adjust_after_replace, adjust_after_replace_noundo)
	(replace_range, replace_range_2, del_range_2): Set CHARS_MODIFF
	to new value of MODIFF.
	* lisp.h (modify_region): Add fourth argument in extern.
	* casefiddle.c (casify_region): Call modify_region with fourth
	argument zero to assert that CHARS_MODIFF is updated.
	* editfns.c (Fsubst_char_in_region, Ftranslate_region_internal)
	(Ftranspose_regions): Likewise.
	* textprop.c (Fadd_text_properties, Fset_text_properties)
	(Fremove_text_properties, Fremove_list_of_text_properties):
	Call modify_region with fourth argument 1 to avoid that
	CHARS_MODIFF is updated.


2006-08-03  Martin Rudalics  <rudalics@gmx.at>

	* imenu.el (imenu-update-menubar): Use `buffer-chars-modified-tick'
	instead of `buffer-modified-tick'.
	(imenu-menubar-modified-tick): Update doc-string accordingly.


*** buffer.c	Tue Jun  6 19:20:40 2006
--- buffer.c	Wed Jul 26 12:30:46 2006
***************
*** 371,376 ****
--- 371,377 ----
    BUF_ZV_BYTE (b) = BEG_BYTE;
    BUF_Z_BYTE (b) = BEG_BYTE;
    BUF_MODIFF (b) = 1;
+   BUF_CHARS_MODIFF (b) = 1;
    BUF_OVERLAY_MODIFF (b) = 1;
    BUF_SAVE_MODIFF (b) = 1;
    BUF_INTERVALS (b) = 0;
***************
*** 1145,1150 ****
--- 1146,1176 ----

    return make_number (BUF_MODIFF (buf));
  }
+ 
+ DEFUN ("buffer-chars-modified-tick", Fbuffer_chars_modified_tick, Sbuffer_chars_modified_tick,
+        0, 1, 0,
+        doc: /* Return BUFFER's character-change tick counter.
+ Each buffer has a character-change tick counter, which is set to the
+ value of the buffer's tick counter \(see `buffer-modified-tick'), each
+ time text in that buffer is inserted or deleted.  By comparing the
+ values returned by two individual calls of buffer-chars-modified-tick,
+ you can tell whether a character change occurred in that buffer in
+ between these calls.  No argument or nil as argument means use current
+ buffer as BUFFER.  */)
+      (buffer)
+      register Lisp_Object buffer;
+ {
+   register struct buffer *buf;
+   if (NILP (buffer))
+     buf = current_buffer;
+   else
+     {
+       CHECK_BUFFER (buffer);
+       buf = XBUFFER (buffer);
+     }
+ 
+   return make_number (BUF_CHARS_MODIFF (buf));
+ }
  \f
  DEFUN ("rename-buffer", Frename_buffer, Srename_buffer, 1, 2,
         "sRename buffer (to new name): \nP",
***************
*** 6014,6019 ****
--- 6040,6046 ----
    defsubr (&Sbuffer_modified_p);
    defsubr (&Sset_buffer_modified_p);
    defsubr (&Sbuffer_modified_tick);
+   defsubr (&Sbuffer_chars_modified_tick);
    defsubr (&Srename_buffer);
    defsubr (&Sother_buffer);
    defsubr (&Sbuffer_enable_undo);

*** buffer.h	Tue Apr 11 16:24:24 2006
--- buffer.h	Wed Jul 26 11:08:36 2006
***************
*** 82,87 ****
--- 82,90 ----
  /* Modification count.  */
  #define MODIFF (current_buffer->text->modiff)

+ /* Character modification count.  */
+ #define CHARS_MODIFF (current_buffer->text->chars_modiff)
+ 
  /* Overlay modification count.  */
  #define OVERLAY_MODIFF (current_buffer->text->overlay_modiff)

***************
*** 147,152 ****
--- 150,158 ----
  /* Modification count.  */
  #define BUF_MODIFF(buf) ((buf)->text->modiff)

+ /* Character modification count.  */
+ #define BUF_CHARS_MODIFF(buf) ((buf)->text->chars_modiff)
+ 
  /* Modification count as of last visit or save.  */
  #define BUF_SAVE_MODIFF(buf) ((buf)->text->save_modiff)

***************
*** 406,411 ****
--- 412,421 ----
  				   for this buffer.  It is incremented for
  				   each such event, and never otherwise
  				   changed.  */
+     int chars_modiff;		/* This is modified with character-change
+ 				   events for this buffer.  It is set to
+ 				   modiff for each such event, and never
+ 				   otherwise changed.  */
      int save_modiff;		/* Previous value of modiff, as of last
  				   time buffer visited or saved a file.  */


*** casefiddle.c	Tue Apr 11 16:24:24 2006
--- casefiddle.c	Tue Jul 25 18:41:12 2006
***************
*** 189,195 ****
    validate_region (&b, &e);
    start = XFASTINT (b);
    end = XFASTINT (e);
!   modify_region (current_buffer, start, end);
    record_change (start, end - start);
    start_byte = CHAR_TO_BYTE (start);
    end_byte = CHAR_TO_BYTE (end);
--- 189,195 ----
    validate_region (&b, &e);
    start = XFASTINT (b);
    end = XFASTINT (e);
!   modify_region (current_buffer, start, end, 0);
    record_change (start, end - start);
    start_byte = CHAR_TO_BYTE (start);
    end_byte = CHAR_TO_BYTE (end);

*** editfns.c	Sun Jul  2 09:50:00 2006
--- editfns.c	Tue Jul 25 18:41:54 2006
***************
*** 2785,2791 ****
  	  if (! changed)
  	    {
  	      changed = pos;
! 	      modify_region (current_buffer, changed, XINT (end));

  	      if (! NILP (noundo))
  		{
--- 2785,2791 ----
  	  if (! changed)
  	    {
  	      changed = pos;
! 	      modify_region (current_buffer, changed, XINT (end), 0);

  	      if (! NILP (noundo))
  		{
***************
*** 2897,2903 ****
    pos = XINT (start);
    pos_byte = CHAR_TO_BYTE (pos);
    end_pos = XINT (end);
!   modify_region (current_buffer, pos, XINT (end));

    cnt = 0;
    for (; pos < end_pos; )
--- 2897,2903 ----
    pos = XINT (start);
    pos_byte = CHAR_TO_BYTE (pos);
    end_pos = XINT (end);
!   modify_region (current_buffer, pos, XINT (end), 0);

    cnt = 0;
    for (; pos < end_pos; )
***************
*** 4162,4168 ****

    if (end1 == start2)		/* adjacent regions */
      {
!       modify_region (current_buffer, start1, end2);
        record_change (start1, len1 + len2);

        tmp_interval1 = copy_intervals (cur_intv, start1, len1);
--- 4162,4168 ----

    if (end1 == start2)		/* adjacent regions */
      {
!       modify_region (current_buffer, start1, end2, 0);
        record_change (start1, len1 + len2);

        tmp_interval1 = copy_intervals (cur_intv, start1, len1);
***************
*** 4218,4225 ****
          {
  	  USE_SAFE_ALLOCA;

!           modify_region (current_buffer, start1, end1);
!           modify_region (current_buffer, start2, end2);
            record_change (start1, len1);
            record_change (start2, len2);
            tmp_interval1 = copy_intervals (cur_intv, start1, len1);
--- 4218,4225 ----
          {
  	  USE_SAFE_ALLOCA;

!           modify_region (current_buffer, start1, end1, 0);
!           modify_region (current_buffer, start2, end2, 0);
            record_change (start1, len1);
            record_change (start2, len2);
            tmp_interval1 = copy_intervals (cur_intv, start1, len1);
***************
*** 4248,4254 ****
          {
  	  USE_SAFE_ALLOCA;

!           modify_region (current_buffer, start1, end2);
            record_change (start1, (end2 - start1));
            tmp_interval1 = copy_intervals (cur_intv, start1, len1);
            tmp_interval_mid = copy_intervals (cur_intv, end1, len_mid);
--- 4248,4254 ----
          {
  	  USE_SAFE_ALLOCA;

!           modify_region (current_buffer, start1, end2, 0);
            record_change (start1, (end2 - start1));
            tmp_interval1 = copy_intervals (cur_intv, start1, len1);
            tmp_interval_mid = copy_intervals (cur_intv, end1, len_mid);
***************
*** 4279,4285 ****
  	  USE_SAFE_ALLOCA;

            record_change (start1, (end2 - start1));
!           modify_region (current_buffer, start1, end2);

            tmp_interval1 = copy_intervals (cur_intv, start1, len1);
            tmp_interval_mid = copy_intervals (cur_intv, end1, len_mid);
--- 4279,4285 ----
  	  USE_SAFE_ALLOCA;

            record_change (start1, (end2 - start1));
!           modify_region (current_buffer, start1, end2, 0);

            tmp_interval1 = copy_intervals (cur_intv, start1, len1);
            tmp_interval_mid = copy_intervals (cur_intv, end1, len_mid);

*** insdel.c	Tue Apr 11 16:24:24 2006
--- insdel.c	Wed Jul 26 17:40:24 2006
***************
*** 1007,1012 ****
--- 1007,1013 ----
       will add up to the right stuff in the undo list.  */
    record_insert (PT, nchars);
    MODIFF++;
+   CHARS_MODIFF = MODIFF;

    bcopy (string, GPT_ADDR, nbytes);

***************
*** 1144,1149 ****
--- 1145,1151 ----

    record_insert (PT, nchars);
    MODIFF++;
+   CHARS_MODIFF = MODIFF;

    GAP_SIZE -= outgoing_nbytes;
    GPT += nchars;
***************
*** 1295,1300 ****
--- 1297,1303 ----

    record_insert (PT, nchars);
    MODIFF++;
+   CHARS_MODIFF = MODIFF;

    GAP_SIZE -= outgoing_nbytes;
    GPT += nchars;
***************
*** 1403,1408 ****
--- 1406,1412 ----
    if (len == 0)
      evaporate_overlays (from);
    MODIFF++;
+   CHARS_MODIFF = MODIFF;
  }

  /* Like adjust_after_replace, but doesn't require PREV_TEXT.
***************
*** 1453,1458 ****
--- 1457,1463 ----
    if (len == 0)
      evaporate_overlays (from);
    MODIFF++;
+   CHARS_MODIFF = MODIFF;
  }

  /* Record undo information, adjust markers and position keepers for an
***************
*** 1645,1650 ****
--- 1650,1656 ----
    CHECK_MARKERS ();

    MODIFF++;
+   CHARS_MODIFF = MODIFF;
    UNGCPRO;

    signal_after_change (from, nchars_del, GPT - from);
***************
*** 1769,1774 ****
--- 1775,1781 ----
    CHECK_MARKERS ();

    MODIFF++;
+   CHARS_MODIFF = MODIFF;
  }
  \f
  /* Delete characters in current buffer
***************
*** 1950,1955 ****
--- 1957,1963 ----
    if (! EQ (current_buffer->undo_list, Qt))
      record_delete (from, deletion);
    MODIFF++;
+   CHARS_MODIFF = MODIFF;

    /* Relocate point as if it were a marker.  */
    if (from < PT)
***************
*** 1990,2001 ****
     character positions START to END.  This checks the read-only
     properties of the region, calls the necessary modification hooks,
     and warns the next redisplay that it should pay attention to that
!    area.  */

  void
! modify_region (buffer, start, end)
       struct buffer *buffer;
!      int start, end;
  {
    struct buffer *old_buffer = current_buffer;

--- 1998,2012 ----
     character positions START to END.  This checks the read-only
     properties of the region, calls the necessary modification hooks,
     and warns the next redisplay that it should pay attention to that
!    area.
! 
!    If PRESERVE_CHARS_MODIFF is non-zero, do not update CHARS_MODIFF.
!    Otherwise set CHARS_MODIFF to the new value of MODIFF.  */

  void
! modify_region (buffer, start, end, preserve_chars_modiff)
       struct buffer *buffer;
!      int start, end, preserve_chars_modiff;
  {
    struct buffer *old_buffer = current_buffer;

***************
*** 2009,2014 ****
--- 2020,2027 ----
    if (MODIFF <= SAVE_MODIFF)
      record_first_change ();
    MODIFF++;
+   if (! preserve_chars_modiff)
+     CHARS_MODIFF = MODIFF;

    buffer->point_before_scroll = Qnil;


*** lisp.h	Tue Jun  6 19:20:40 2006
--- lisp.h	Tue Jul 25 18:42:54 2006
***************
*** 2451,2457 ****
  extern void del_range_byte P_ ((int, int, int));
  extern void del_range_both P_ ((int, int, int, int, int));
  extern Lisp_Object del_range_2 P_ ((int, int, int, int, int));
! extern void modify_region P_ ((struct buffer *, int, int));
  extern void prepare_to_modify_buffer P_ ((int, int, int *));
  extern void signal_before_change P_ ((int, int, int *));
  extern void signal_after_change P_ ((int, int, int));
--- 2451,2457 ----
  extern void del_range_byte P_ ((int, int, int));
  extern void del_range_both P_ ((int, int, int, int, int));
  extern Lisp_Object del_range_2 P_ ((int, int, int, int, int));
! extern void modify_region P_ ((struct buffer *, int, int, int));
  extern void prepare_to_modify_buffer P_ ((int, int, int *));
  extern void signal_before_change P_ ((int, int, int *));
  extern void signal_after_change P_ ((int, int, int));

*** textprop.c	Sun Jul  2 09:50:00 2006
--- textprop.c	Tue Jul 25 18:45:36 2006
***************
*** 1247,1253 ****
      }

    if (BUFFERP (object))
!     modify_region (XBUFFER (object), XINT (start), XINT (end));

    /* We are at the beginning of interval I, with LEN chars to scan.  */
    for (;;)
--- 1247,1253 ----
      }

    if (BUFFERP (object))
!     modify_region (XBUFFER (object), XINT (start), XINT (end), 1);

    /* We are at the beginning of interval I, with LEN chars to scan.  */
    for (;;)
***************
*** 1387,1393 ****
      }

    if (BUFFERP (object))
!     modify_region (XBUFFER (object), XINT (start), XINT (end));

    set_text_properties_1 (start, end, properties, object, i);

--- 1387,1393 ----
      }

    if (BUFFERP (object))
!     modify_region (XBUFFER (object), XINT (start), XINT (end), 1);

    set_text_properties_1 (start, end, properties, object, i);

***************
*** 1535,1541 ****
      }

    if (BUFFERP (object))
!     modify_region (XBUFFER (object), XINT (start), XINT (end));

    /* We are at the beginning of an interval, with len to scan */
    for (;;)
--- 1535,1541 ----
      }

    if (BUFFERP (object))
!     modify_region (XBUFFER (object), XINT (start), XINT (end), 1);

    /* We are at the beginning of an interval, with len to scan */
    for (;;)
***************
*** 1649,1655 ****
  	  if (LENGTH (i) == len)
  	    {
  	      if (!modified && BUFFERP (object))
! 		modify_region (XBUFFER (object), XINT (start), XINT (end));
  	      remove_properties (Qnil, properties, i, object);
  	      if (BUFFERP (object))
  		signal_after_change (XINT (start), XINT (end) - XINT (start),
--- 1649,1655 ----
  	  if (LENGTH (i) == len)
  	    {
  	      if (!modified && BUFFERP (object))
! 		modify_region (XBUFFER (object), XINT (start), XINT (end), 1);
  	      remove_properties (Qnil, properties, i, object);
  	      if (BUFFERP (object))
  		signal_after_change (XINT (start), XINT (end) - XINT (start),
***************
*** 1662,1668 ****
  	  i = split_interval_left (i, len);
  	  copy_properties (unchanged, i);
  	  if (!modified && BUFFERP (object))
! 	    modify_region (XBUFFER (object), XINT (start), XINT (end));
  	  remove_properties (Qnil, properties, i, object);
  	  if (BUFFERP (object))
  	    signal_after_change (XINT (start), XINT (end) - XINT (start),
--- 1662,1668 ----
  	  i = split_interval_left (i, len);
  	  copy_properties (unchanged, i);
  	  if (!modified && BUFFERP (object))
! 	    modify_region (XBUFFER (object), XINT (start), XINT (end), 1);
  	  remove_properties (Qnil, properties, i, object);
  	  if (BUFFERP (object))
  	    signal_after_change (XINT (start), XINT (end) - XINT (start),
***************
*** 1673,1679 ****
        if (interval_has_some_properties_list (properties, i))
  	{
  	  if (!modified && BUFFERP (object))
! 	    modify_region (XBUFFER (object), XINT (start), XINT (end));
  	  remove_properties (Qnil, properties, i, object);
  	  modified = 1;
  	}
--- 1673,1679 ----
        if (interval_has_some_properties_list (properties, i))
  	{
  	  if (!modified && BUFFERP (object))
! 	    modify_region (XBUFFER (object), XINT (start), XINT (end), 1);
  	  remove_properties (Qnil, properties, i, object);
  	  modified = 1;
  	}

*** imenu.el	Mon May  1 10:08:52 2006
--- imenu.el	Wed Jul 26 11:20:28 2006
***************
*** 968,982 ****
  (defvar imenu-buffer-menubar nil)

  (defvar imenu-menubar-modified-tick 0
!   "The value of (buffer-modified-tick) as of last call to `imenu-update-menubar'.")
  (make-variable-buffer-local 'imenu-menubar-modified-tick)

  (defun imenu-update-menubar ()
    (when (and (current-local-map)
  	     (keymapp (lookup-key (current-local-map) [menu-bar index]))
! 	     (not (eq (buffer-modified-tick)
! 		      imenu-menubar-modified-tick)))
!     (setq imenu-menubar-modified-tick (buffer-modified-tick))
      (let ((index-alist (imenu--make-index-alist t)))
        ;; Don't bother updating if the index-alist has not changed
        ;; since the last time we did it.
--- 968,981 ----
  (defvar imenu-buffer-menubar nil)

  (defvar imenu-menubar-modified-tick 0
!   "The value of (buffer-chars-modified-tick) as of last call to `imenu-update-menubar'.")
  (make-variable-buffer-local 'imenu-menubar-modified-tick)

  (defun imenu-update-menubar ()
    (when (and (current-local-map)
  	     (keymapp (lookup-key (current-local-map) [menu-bar index]))
! 	     (/= (buffer-chars-modified-tick) imenu-menubar-modified-tick))
!     (setq imenu-menubar-modified-tick (buffer-chars-modified-tick))
      (let ((index-alist (imenu--make-index-alist t)))
        ;; Don't bother updating if the index-alist has not changed
        ;; since the last time we did it.


[-- Attachment #3: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

Re: jit lock sit-for provokes redisplay provokes imenu

[Chong Yidong]

martin rudalics <rudalics@gmx.at> writes:

> Attached find ChangeLog entries and patches for this.

I noticed this hasn't been committed.  FWIW, I've looked through the
changes and reread the emacs-devel thread, and the patch looks OK to
me, with one nitpick:

> + values returned by two individual calls of buffer-chars-modified-tick,

This should be in quotes: `buffer-chars-modified-tick'

This assumes that the patch does indeed solve the original problem
(too many imenu updates during redisplay).  I haven't been able to
reproduce this problem, but presumably someone can.

Re: jit lock sit-for provokes redisplay provokes imenu

[Kim F. Storm]

Chong Yidong <cyd@stupidchicken.com> writes:

> martin rudalics <rudalics@gmx.at> writes:
>
>> Attached find ChangeLog entries and patches for this.
>
> I noticed this hasn't been committed.  FWIW, I've looked through the
> changes and reread the emacs-devel thread, and the patch looks OK to
> me, with one nitpick:
>
>> + values returned by two individual calls of buffer-chars-modified-tick,
>
> This should be in quotes: `buffer-chars-modified-tick'
>
> This assumes that the patch does indeed solve the original problem
> (too many imenu updates during redisplay).  I haven't been able to
> reproduce this problem, but presumably someone can.

Is this change still needed for fixing the original problem if we
completely eliminate sit-for in jit-lock?

OTOH, adding buffer-chars-modified-tick in itself (and the change to
imenu)still seems like a good thing, and I also think the change is
safe.   But we need updates for lispref and NEWS.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: jit lock sit-for provokes redisplay provokes imenu

[Chong Yidong]

storm@cua.dk (Kim F. Storm) writes:

>> This assumes that the patch does indeed solve the original problem
>> (too many imenu updates during redisplay).  I haven't been able to
>> reproduce this problem, but presumably someone can.
>
> Is this change still needed for fixing the original problem if we
> completely eliminate sit-for in jit-lock?

I don't think the jit-lock change will resolve this particular
problem, which is that the text property changes during each jit-lock
fontification leads to excessive imenu updates.

> OTOH, adding buffer-chars-modified-tick in itself (and the change to
> imenu)still seems like a good thing, and I also think the change is
> safe.   But we need updates for lispref and NEWS.

Yep.

Re: jit lock sit-for provokes redisplay provokes imenu

[Kim F. Storm]

Chong Yidong <cyd@stupidchicken.com> writes:

> storm@cua.dk (Kim F. Storm) writes:
>
>>> This assumes that the patch does indeed solve the original problem
>>> (too many imenu updates during redisplay).  I haven't been able to
>>> reproduce this problem, but presumably someone can.
>>
>> Is this change still needed for fixing the original problem if we
>> completely eliminate sit-for in jit-lock?
>
> I don't think the jit-lock change will resolve this particular
> problem, which is that the text property changes during each jit-lock
> fontification leads to excessive imenu updates.
>
>> OTOH, adding buffer-chars-modified-tick in itself (and the change to
>> imenu)still seems like a good thing, and I also think the change is
>> safe.   But we need updates for lispref and NEWS.
>
> Yep.

I haven't heard any objections, so could you pls. install the patch
and write something for lispref and NEWS.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk