[spiegel@gnu.org: Re: [Fwd: vc-annotate causes Emacs to die]]

[spiegel@gnu.org: Re: [Fwd: vc-annotate causes Emacs to die]]

[Richard Stallman]

Can you help fix this bug in redisplay?

------- Start of forwarded message -------
Subject: Re: [Fwd: vc-annotate causes Emacs to die]
From: Andre Spiegel <spiegel@gnu.org>
To: Stefan Monnier <monnier@cs.yale.edu>
Cc: rms@gnu.org, bob@rattlesnake.com, billw@wolfram.com
In-Reply-To: <200208090636.g796aNY11313@rum.cs.yale.edu>
Date: 23 Aug 2002 14:37:02 +0200
X-Spam-Status: No, hits=-3.1 required=5.0
	tests=IN_REP_TO,WEIRD_PORT
	version=2.31
X-Spam-Level: 

Hi Stefan,

since the time when you made vc-annotate use jit-lock, using it causes a
segfault for many users.  You wrote:

> It's unrelated to VC since it's a bug in the C code.

This is of course true, but vc-annotate still used to work prior to your
modification.  I'm not qualified to have a look at the C level problem,
and it would take me a lot of time to catch up on this.  Can you point
me to someone who would be more qualified to do it (I'm including the
segfault stack trace below)?  If not, I suggest reverting vc-annotate to
the way it was before your change, until somebody finds the time to fix
the C code.

Here's the stack trace (taken from Bob's message):

#0  0x080aad1e in x_fill_glyph_string (s=0xbfffe874, face_id=12,
start=35, 
    end=80, overlaps_p=0) at xterm.c:1420
#1  0x080ab615 in x_draw_glyphs (w=0x82d53e8, x=305, row=0x84f7b9c, 
    area=TEXT_AREA, start=0, end=80, hl=DRAW_NORMAL_TEXT, overlaps_p=0)
    at xterm.c:5142
#2  0x080adee6 in x_write_glyphs (start=0x8528a98, len=80) at
xterm.c:5339
#3  0x08053c4c in update_text_area (w=0x82d53e8, vpos=3) at
dispnew.c:4265
#4  0x08054082 in update_window_line (w=0x82d53e8, vpos=3, 
    mouse_face_overwritten_p=0xbfffec68) at dispnew.c:4489
#5  0x080539eb in update_window (w=0x82d53e8, force_p=0) at
dispnew.c:4145
#6  0x0805347b in update_window_tree (w=0x84bb258, force_p=0) at
dispnew.c:3879
#7  0x08053466 in update_window_tree (w=0x84c4420, force_p=0) at
dispnew.c:3877
#8  0x08053385 in update_frame (f=0x84484b0, force_p=0,
inhibit_hairy_id_p=0)
    at dispnew.c:3816
#9  0x08067224 in redisplay_internal (preserve_echo_area=0) at
xdisp.c:8964
#10 0x0806617c in redisplay () at xdisp.c:8374
#11 0x080d6075 in read_char (commandflag=1, nmaps=2, maps=0xbffff424, 
    prev_event=405373956, used_mouse_menu=0xbffff470) at keyboard.c:2323
#12 0x080dccb0 in read_key_sequence (keybuf=0xbffff574, bufsize=30, 
    prompt=405373956, dont_downcase_last=0, can_return_switch_frame=1, 
    fix_current_buffer=1) at keyboard.c:8363
#13 0x080d4be8 in command_loop_1 () at keyboard.c:1470
#14 0x08128159 in internal_condition_case (bfun=0x80d48e0
    <command_loop_1>
    handlers=405470516, hfun=0x80d44f4 <cmd_error>) at eval.c:1349
#15 0x080d47b8 in command_loop_2 () at keyboard.c:1271
#16 0x08127ced in internal_catch (tag=405431844, 
    func=0x80d4794 <command_loop_2>, arg=405373956) at eval.c:1109
#17 0x080d4763 in command_loop () at keyboard.c:1250
#18 0x080d42b0 in recursive_edit_1 () at keyboard.c:966
#19 0x080d43e0 in Frecursive_edit () at keyboard.c:1022
#20 0x080d32b3 in main (argc=5, argv=0xbffffb34, envp=0xbffffb4c)
    at emacs.c:1625
------- End of forwarded message -------

Re: [spiegel@gnu.org: Re: [Fwd: vc-annotate causes Emacs to die]]

[Gerd Moellmann]

Richard Stallman <rms@gnu.org> writes:

> Can you help fix this bug in redisplay?
> 
> From: Andre Spiegel <spiegel@gnu.org>
> Subject: Re: [Fwd: vc-annotate causes Emacs to die]
> To: Stefan Monnier <monnier@cs.yale.edu>
> Cc: rms@gnu.org, bob@rattlesnake.com, billw@wolfram.com
> Date: 23 Aug 2002 14:37:02 +0200
> 
> Hi Stefan,
> 
> since the time when you made vc-annotate use jit-lock, using it causes a
> segfault for many users.  You wrote:
> 
> > It's unrelated to VC since it's a bug in the C code.
> 
> This is of course true, but vc-annotate still used to work prior to your
> modification. 

Can someone tell me what modification that is?

What's the recipe to reproduce this?  And with which version of Emacs?

Re: vc-annotate causes Emacs to die

[Andre Spiegel]

On Mon, 2002-08-26 at 15:33, Gerd Moellmann wrote:

> Can someone tell me what modification that is?
> 
> What's the recipe to reproduce this?  And with which version of Emacs?

On 2002-07-16, Stefan changed vc-annotate to use jit-lock.  Since then,
vc-annotate (C-x v g) on a CVS-controlled file seems to fail with a
segfault for several people.  For my own part, I'm getting a segfault
all the time, even with -q --no-sitefile.  This is current CVS Emacs,
with a dark background and orange text using a neep font.

Re: [spiegel@gnu.org: Re: [Fwd: vc-annotate causes Emacs to die]]

[Alan Shutko]

gerd.moellmann@t-online.de (Gerd Moellmann) writes:

> What's the recipe to reproduce this?  And with which version of Emacs?

Here's a recipe.  Current CVS Emacs, in the Emacs CVS directory.

Script started on Mon Aug 26 14:32:50 2002
[14:32:50] wesley:~/Projects/EmacsBugs/emacs $ pwd
/home/ats/Projects/EmacsBugs/emacs
[14:33:06] wesley:~/Projects/EmacsBugs/emacs $ emacs -q --no-site-file ChangeLog
[In Emacs, hit C-x v g ]

Fatal error (11).Segmentation fault (core dumped)
[14:33:22] wesley:~/Projects/EmacsBugs/emacs $ gdb `which emacs` core.10812 
GNU gdb 2002-08-18-cvs
Copyright 2002 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and you are
welcome to change it and/or distribute copies of it under certain conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB.  Type "show warranty" for details.
This GDB was configured as "i386-linux"...
Core was generated by `emacs -q --no-site-file ChangeLog'.
Program terminated with signal 11, Segmentation fault.
Reading symbols from /usr/X11R6/lib/libXaw3d.so.6...done.
Loaded symbols for /usr/X11R6/lib/libXaw3d.so.6
Reading symbols from /usr/X11R6/lib/libXmu.so.6...done.
Loaded symbols for /usr/X11R6/lib/libXmu.so.6
Reading symbols from /usr/X11R6/lib/libXt.so.6...done.
Loaded symbols for /usr/X11R6/lib/libXt.so.6
Reading symbols from /usr/X11R6/lib/libSM.so.6...done.
Loaded symbols for /usr/X11R6/lib/libSM.so.6
Reading symbols from /usr/X11R6/lib/libICE.so.6...done.
Loaded symbols for /usr/X11R6/lib/libICE.so.6
Reading symbols from /usr/X11R6/lib/libXext.so.6...done.
Loaded symbols for /usr/X11R6/lib/libXext.so.6
Reading symbols from /usr/lib/libtiff.so.3...done.
Loaded symbols for /usr/lib/libtiff.so.3
Reading symbols from /usr/lib/libjpeg.so.62...done.
Loaded symbols for /usr/lib/libjpeg.so.62
Reading symbols from /usr/lib/libpng.so.3...done.
Loaded symbols for /usr/lib/libpng.so.3
Reading symbols from /usr/lib/libz.so.1...done.
Loaded symbols for /usr/lib/libz.so.1
Reading symbols from /lib/libm.so.6...done.
Loaded symbols for /lib/libm.so.6
Reading symbols from /usr/lib/libungif.so.4...done.
Loaded symbols for /usr/lib/libungif.so.4
Reading symbols from /usr/X11R6/lib/libXpm.so.4...done.
Loaded symbols for /usr/X11R6/lib/libXpm.so.4
Reading symbols from /usr/X11R6/lib/libX11.so.6...done.
Loaded symbols for /usr/X11R6/lib/libX11.so.6
---Type <return> to continue, or q <return> to quit---b\b \b
Reading symbols from /lib/libncurses.so.5...done.
Loaded symbols for /lib/libncurses.so.5
Reading symbols from /lib/libc.so.6...done.
Loaded symbols for /lib/libc.so.6
Reading symbols from /lib/libdl.so.2...done.
Loaded symbols for /lib/libdl.so.2
Reading symbols from /lib/ld-linux.so.2...done.
Loaded symbols for /lib/ld-linux.so.2
Reading symbols from /usr/X11R6/lib/X11/locale/common/xlcDef.so.2...done.
Loaded symbols for /usr/X11R6/lib/X11/locale/common/xlcDef.so.2
Reading symbols from /usr/X11R6/lib/X11/locale/common/ximcp.so.2...done.
Loaded symbols for /usr/X11R6/lib/X11/locale/common/ximcp.so.2
Reading symbols from /usr/lib/gconv/ISO8859-1.so...done.
Loaded symbols for /usr/lib/gconv/ISO8859-1.so
Reading symbols from /usr/X11R6/lib/X11/locale/common/xomGeneric.so.2...done.
Loaded symbols for /usr/X11R6/lib/X11/locale/common/xomGeneric.so.2
#0  0x402ea581 in kill () from /lib/libc.so.6
(gdb) bt
#0  0x402ea581 in kill () from /lib/libc.so.6
#1  0x080d5ec9 in fatal_error_signal (sig=11) at emacs.c:378
#2  0x402ea518 in sigaction () from /lib/libc.so.6
#3  0x080ae314 in x_draw_glyphs (w=0x857cfa0, x=305, row=<incomplete type>, 
    area=TEXT_AREA, start=0, end=76, hl=DRAW_NORMAL_TEXT, overlaps_p=0)
    at xterm.c:5144
#4  0x080b0c87 in x_write_glyphs (start=<incomplete type>, len=76)
    at xterm.c:5341
#5  0x0805430e in update_text_area (w=0x857cfa0, vpos=2) at dispnew.c:4265
#6  0x08054754 in update_window_line (w=0x857cfa0, vpos=2, 
    mouse_face_overwritten_p=0xbfffe958) at dispnew.c:4489
#7  0x080540bf in update_window (w=0x857cfa0, force_p=0) at dispnew.c:4145
#8  0x08053b63 in update_window_tree (w=0x84f9730, force_p=0) at dispnew.c:3879
#9  0x08053b4d in update_window_tree (w=0x85cd300, force_p=0) at dispnew.c:3877
#10 0x08053a5c in update_frame (f=0x84f9608, force_p=0, inhibit_hairy_id_p=0)
    at dispnew.c:3816
#11 0x08067e90 in redisplay_internal (preserve_echo_area=0) at xdisp.c:8968
#12 0x08066dbc in redisplay () at xdisp.c:8377
#13 0x080da0f5 in read_char (commandflag=1, nmaps=3, maps=0xbffff104, 
    prev_event=405490116, used_mouse_menu=0xbffff150) at keyboard.c:2325
#14 0x080e142f in read_key_sequence (keybuf=0xbffff254, bufsize=30, 
    prompt=405490116, dont_downcase_last=0, can_return_switch_frame=1, 
    fix_current_buffer=1) at keyboard.c:8368
---Type <return> to continue, or q <return> to quit---
#15 0x080d8b2b in command_loop_1 () at keyboard.c:1472
#16 0x0812f139 in internal_condition_case (bfun=0x80d8804 <command_loop_1>, 
    handlers=405586700, hfun=0x80d8414 <cmd_error>) at eval.c:1348
#17 0x080d86d8 in command_loop_2 () at keyboard.c:1273
#18 0x0812eccd in internal_catch (tag=405548004, 
    func=0x80d86b4 <command_loop_2>, arg=405490116) at eval.c:1108
#19 0x080d8687 in command_loop () at keyboard.c:1252
#20 0x080d81d8 in recursive_edit_1 () at keyboard.c:968
#21 0x080d8300 in Frecursive_edit () at keyboard.c:1024
#22 0x080d7168 in main (argc=4, argv=0xbffff814, envp=0xbffff828)
    at emacs.c:1630
(gdb) [14:34:10] wesley:~/Projects/EmacsBugs/emacs $ 
Script done on Mon Aug 26 14:34:12 2002

System info: Debian GNU/Linux Unstable

-- 
Alan Shutko <ats@acm.org> - In a variety of flavors!
There's obviously been some sort of silly mistake.

Re: [spiegel@gnu.org: Re: [Fwd: vc-annotate causes Emacs to die]]

[Gerd Moellmann]

Alan Shutko <ats@acm.org> writes:

> gerd.moellmann@t-online.de (Gerd Moellmann) writes:
> 
> > What's the recipe to reproduce this?  And with which version of Emacs?
> 
> Here's a recipe.  Current CVS Emacs, in the Emacs CVS directory.
> 
> Script started on Mon Aug 26 14:32:50 2002
> [14:32:50] wesley:~/Projects/EmacsBugs/emacs $ pwd
> /home/ats/Projects/EmacsBugs/emacs
> [14:33:06] wesley:~/Projects/EmacsBugs/emacs $ emacs -q --no-site-file ChangeLog
> [In Emacs, hit C-x v g ]

Thanks, Alan & Andre.  I can reproduce this with Alan's recipe.

Until I find time to debug this further, it might be worth using this
quich workaround:

Index: vc.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/vc.el,v
retrieving revision 1.335
diff -c -u -r1.335 vc.el
cvs server: conflicting specifications of output style
--- vc.el	22 Jul 2002 18:52:04 -0000	1.335
+++ vc.el	26 Aug 2002 20:07:37 -0000
@@ -3219,16 +3219,10 @@
 		(setq difference (vc-annotate-difference vc-annotate-offset)))
       (let* ((color (or (vc-annotate-compcar difference vc-annotate-color-map)
 			(cons nil vc-annotate-very-old-color)))
-	     ;; substring from index 1 to remove any leading `#' in the name
-	     (face-name (concat "vc-annotate-face-" (substring (cdr color) 1)))
 	     ;; Make the face if not done.
-	     (face (or (intern-soft face-name)
-		       (let ((tmp-face (make-face (intern face-name))))
-			 (set-face-foreground tmp-face (cdr color))
-			 (if vc-annotate-background
-			     (set-face-background tmp-face
-						  vc-annotate-background))
-			 tmp-face)))	; Return the face
+	     (face (nconc (list :foreground (cdr color))
+			  (when vc-annotate-background
+			    (list :background vc-annotate-background))))
 	     (point (point))
 	     overlay)
 	(forward-line 1)

(This uses anonymous faces instead of constructing new named faces.
(My personal impression is that using anonymous faces might be
preferrable here, anyway.))

The original problem is that one of the set-face-{back,fore}ground
calls apparently leads to the face cache being cleared, which leaves
redisplay with a nonexistent face referenced from a glyph.  When
compiled with -DGLYPH_DEBUG, this results in an assertion violation.

Under which exact circumstances this face cache clearing happens is
not yet clear to me.  I think this cannot happen during redisplay,
since that is explicitly prevented by setting a flag.  Hm.

Re: [spiegel@gnu.org: Re: [Fwd: vc-annotate causes Emacs to die]]

[Gerd Moellmann]

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

Gerd Moellmann <gerd.moellmann@t-online.de> writes:

> Until I find time to debug this further, it might be worth using this
> quich workaround:

[...]

I think the real fix could be what's in the attached patch (please
install when tested).

P.S.

Can I now have the 21.2 cursor shapes back, as I requested a couple of
times?  The new cursor shapes for the off-state of the blinking cursor
and in non-selected windows drive me up the wall.  Why was that change
made, for heaven's sake?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: d.diff --]
[-- Type: text/x-patch, Size: 4796 bytes --]

2002-08-26  Gerd Moellmann  <gerd.moellmann@t-online.de>

	* xdisp.c (redisplay_updating_p): Variable removed.
	(inhibit_free_realized_faces, Qinhibit_free_realized_faces): New
	variables.
	(init_iterator): Don't free realized faces if
	inhibit_free_realized_faces is set.
	(redisplay_internal): Bind Qinhibit_free_realized_faces to nil.
	(syms_of_xdisp): DEFVAR_BOOL inhibit-free-realized-faces,
	initialize Qinhibit_free_realized_faces.

	* dispextern.h (PRODUCE_GLYPHS): Set inhibit_free_realized_faces
	when iterator is adding glyphs to a glyph matrix.

Index: dispextern.h
===================================================================
RCS file: /cvsroot/emacs/emacs/src/dispextern.h,v
retrieving revision 1.137
diff -c -u -r1.137 dispextern.h
cvs server: conflicting specifications of output style
--- dispextern.h	16 Jul 2002 19:48:08 -0000	1.137
+++ dispextern.h	26 Aug 2002 21:43:53 -0000
@@ -1852,10 +1852,16 @@
 /* Call produce_glyphs or produce_glyphs_hook, if set.  Shortcut to
    avoid the function call overhead.  */
 
-#define PRODUCE_GLYPHS(IT)			\
-     (rif					\
-      ? rif->produce_glyphs ((IT))		\
-      : produce_glyphs ((IT)))
+#define PRODUCE_GLYPHS(IT) 			\
+     do {					\
+       extern int inhibit_free_realized_faces;	\
+       if (rif != NULL)				\
+	 rif->produce_glyphs ((IT));		\
+       else					\
+	 produce_glyphs ((IT));			\
+       if ((IT)->glyph_row != NULL)		\
+	 inhibit_free_realized_faces = 1;	\
+     } while (0)
 
 /* Bit-flags indicating what operation move_it_to should perform.  */
 
Index: xdisp.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/xdisp.c,v
retrieving revision 1.777
diff -c -u -r1.777 xdisp.c
cvs server: conflicting specifications of output style
--- xdisp.c	22 Aug 2002 16:52:56 -0000	1.777
+++ xdisp.c	26 Aug 2002 21:45:10 -0000
@@ -706,9 +706,12 @@
 
 int redisplaying_p;
 
-/* Non-zero while redisplay is updating the display.  */
+/* Non-zero means don't free realized faces.  Bound while freeing
+   realized faces is dangerous because glyph matrices might still
+   reference them.  */
 
-int redisplay_updating_p;
+int inhibit_free_realized_faces;
+Lisp_Object Qinhibit_free_realized_faces;
 
 \f
 /* Function prototypes.  */
@@ -1524,7 +1527,7 @@
      free realized faces now because they depend on face definitions
      that might have changed.  Don't free faces while there might be 
      desired matrices pending which reference these faces.  */
-  if (face_change_count && !redisplay_updating_p)
+  if (face_change_count && !inhibit_free_realized_faces)
     {
       face_change_count = 0;
       free_all_realized_faces (Qnil);
@@ -8514,11 +8517,11 @@
   count = SPECPDL_INDEX ();
   record_unwind_protect (unwind_redisplay, make_number (redisplaying_p));
   ++redisplaying_p;
+  specbind (Qinhibit_free_realized_faces, Qnil);
 
  retry:
   pause = 0;
   reconsider_clip_changes (w, current_buffer);
-  redisplay_updating_p = 0;
 
   /* If new fonts have been loaded that make a glyph matrix adjustment
      necessary, do it.  */
@@ -9006,9 +9009,8 @@
 				 redisplay_window_error);
 
       /* Compare desired and current matrices, perform output.  */
-    update:
-      redisplay_updating_p = 1;
       
+    update:
       /* If fonts changed, display again.  */
       if (fonts_changed_p)
 	goto retry;
@@ -9134,7 +9136,6 @@
     goto retry;
 
  end_of_redisplay:
-  redisplay_updating_p = 0;
   unbind_to (count, Qnil);
 }
 
@@ -9172,14 +9173,13 @@
 /* Function registered with record_unwind_protect in
    redisplay_internal.  Reset redisplaying_p to the value it had
    before redisplay_internal was called, and clear
-   redisplay_updating_p.  */
+   prevent_freeing_realized_faces_p.  */
 
 static Lisp_Object
 unwind_redisplay (old_redisplaying_p)
      Lisp_Object old_redisplaying_p;
 {
   redisplaying_p = XFASTINT (old_redisplaying_p);
-  redisplay_updating_p = 0;
   return Qnil;
 }
 
@@ -15299,6 +15299,8 @@
   staticpro (&Qobject);
   Qrisky_local_variable = intern ("risky-local-variable");
   staticpro (&Qrisky_local_variable);
+  Qinhibit_free_realized_faces = intern ("inhibit-free-realized-faces");
+  staticpro (&Qinhibit_free_realized_faces);
 
   list_of_error = Fcons (intern ("error"), Qnil);
   staticpro (&list_of_error);
@@ -15560,6 +15562,10 @@
   DEFVAR_BOOL ("inhibit-eval-during-redisplay", &inhibit_eval_during_redisplay,
     doc: /* Non-nil means don't eval Lisp during redisplay.  */);
   inhibit_eval_during_redisplay = 0;
+
+  DEFVAR_BOOL ("inhibit-free-realized-faces", &inhibit_free_realized_faces,
+    doc: /* Non-nil means don't free realized faces.  Internal use only.  */);
+  inhibit_free_realized_faces = 0;
 
 #if GLYPH_DEBUG
   DEFVAR_BOOL ("inhibit-try-window-id", &inhibit_try_window_id,

Re: [spiegel@gnu.org: Re: [Fwd: vc-annotate causes Emacs to die]]

[Miles Bader]

gerd.moellmann@t-online.de (Gerd Moellmann) writes:
> Can I now have the 21.2 cursor shapes back, as I requested a couple of
> times?  The new cursor shapes for the off-state of the blinking cursor
> and in non-selected windows drive me up the wall.

I agree, the old style [of blinking cursor off-state] was better (it
doesn't obscure characters).  Weren't cursors in non-selected
windows the same before though (not that I like them; I have
cursor-in-non-selected-windows set to nil)? 

-Miles
-- 
97% of everything is grunge

Re: [spiegel@gnu.org: Re: [Fwd: vc-annotate causes Emacs to die]]

[Gerd Moellmann]

Miles Bader <miles@lsi.nec.co.jp> writes:

> gerd.moellmann@t-online.de (Gerd Moellmann) writes:
> > Can I now have the 21.2 cursor shapes back, as I requested a couple of
> > times?  The new cursor shapes for the off-state of the blinking cursor
> > and in non-selected windows drive me up the wall.
> 
> I agree, the old style [of blinking cursor off-state] was better (it
> doesn't obscure characters).  

Exactly.

> Weren't cursors in non-selected
> windows the same before though (not that I like them; I have
> cursor-in-non-selected-windows set to nil)? 

The cursor shape for a bar cursor in non-selected windows has become a
thin bar cursor.  Default: 2 pixels in selected window, 1 pixel in
other windows.

Re: [spiegel@gnu.org: Re: [Fwd: vc-annotate causes Emacs to die]]

[Gerd Moellmann]

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

> > Can I now have the 21.2 cursor shapes back, as I requested a couple of
> > times?  The new cursor shapes for the off-state of the blinking cursor
> > and in non-selected windows drive me up the wall.  Why was that change
> > made, for heaven's sake?
> > 
> 
> Try this patch [X version only, but easily ported to mac and w32].
> 
> It changes cursor-in-non-selected-windows to a lisp object with
> (almost) same values as cursor-type, and adds alternative-cursor-type
> for the "off-state" of the blinking cursor.

Yes, that works nicely.  Thank you very much for writing this, Kim.

May I propose to add X resource names for the cursor types analogous
to `cursorType' (xfns.c, function x_window, I believe)?  And it would be
extra nice if cursor-in-non-selected-windows could be set to
`hollow-box', too.

Re: [spiegel@gnu.org: Re: [Fwd: vc-annotate causes Emacs to die]]

[Kim F. Storm]

gerd.moellmann@t-online.de (Gerd Moellmann) writes:

> Gerd Moellmann <gerd.moellmann@t-online.de> writes:
> 
> P.S.
> 
> Can I now have the 21.2 cursor shapes back, as I requested a couple of
> times?  The new cursor shapes for the off-state of the blinking cursor
> and in non-selected windows drive me up the wall.  Why was that change
> made, for heaven's sake?
> 

Try this patch [X version only, but easily ported to mac and w32].

It changes cursor-in-non-selected-windows to a lisp object with
(almost) same values as cursor-type, and adds alternative-cursor-type
for the "off-state" of the blinking cursor.


Anyone else think this is nice to have?



Index: dispextern.h
===================================================================
RCS file: /cvs/emacs/src/dispextern.h,v
retrieving revision 1.137
diff -c -r1.137 dispextern.h
*** dispextern.h	16 Jul 2002 19:48:08 -0000	1.137
--- dispextern.h	27 Aug 2002 14:48:45 -0000
***************
*** 2268,2274 ****
  extern void add_to_log P_ ((char *, Lisp_Object, Lisp_Object));
  extern int help_echo_showing_p;
  extern int current_mode_line_height, current_header_line_height;
! extern int cursor_in_non_selected_windows;
  
  /* Defined in sysdep.c */
  
--- 2268,2275 ----
  extern void add_to_log P_ ((char *, Lisp_Object, Lisp_Object));
  extern int help_echo_showing_p;
  extern int current_mode_line_height, current_header_line_height;
! extern Lisp_Object Vcursor_in_non_selected_windows;
! extern Lisp_Object Valternate_cursor_type;
  
  /* Defined in sysdep.c */
  
Index: lisp.h
===================================================================
RCS file: /cvs/emacs/src/lisp.h,v
retrieving revision 1.438
diff -c -r1.438 lisp.h
*** lisp.h	14 Aug 2002 10:35:03 -0000	1.438
--- lisp.h	27 Aug 2002 14:48:45 -0000
***************
*** 2364,2369 ****
--- 2364,2370 ----
  extern Lisp_Object Qinhibit_eval_during_redisplay;
  extern Lisp_Object Qmessage_truncate_lines;
  extern Lisp_Object Qcursor_in_non_selected_windows;
+ extern Lisp_Object Qalternate_cursor_type;
  extern Lisp_Object Vmessage_log_max;
  extern int message_enable_multibyte;
  extern Lisp_Object echo_area_buffer[2];
Index: xdisp.c
===================================================================
RCS file: /cvs/emacs/src/xdisp.c,v
retrieving revision 1.778
diff -c -r1.778 xdisp.c
*** xdisp.c	27 Aug 2002 00:59:55 -0000	1.778
--- xdisp.c	27 Aug 2002 14:48:46 -0000
***************
*** 528,539 ****
  
  static int message_cleared_p;
  
! /* Non-zero means we want a hollow cursor in windows that are not
!    selected.  Zero means there's no cursor in such windows.  */
  
! int cursor_in_non_selected_windows;
  Lisp_Object Qcursor_in_non_selected_windows;
  
  /* A scratch glyph row with contents used for generating truncation
     glyphs.  Also used in direct_output_for_insert.  */
  
--- 528,547 ----
  
  static int message_cleared_p;
  
! /* Specifies the cursor type to display in windows that are not
!    selected.  t means we want a hollow cursor in such windows.
!    nil means there's no cursor in such windows.  */
  
! Lisp_Object Vcursor_in_non_selected_windows;
  Lisp_Object Qcursor_in_non_selected_windows;
  
+ /* Specifies the desired cursor-type to use to show the blinking
+    cursor off state and cursor shown in non-selected windows.
+    t means to use the default.  */
+ 
+ Lisp_Object Valternate_cursor_type;
+ Lisp_Object Qalternate_cursor_type;
+ 
  /* A scratch glyph row with contents used for generating truncation
     glyphs.  Also used in direct_output_for_insert.  */
  
***************
*** 15293,15298 ****
--- 15301,15308 ----
    staticpro (&Qmessage_truncate_lines);
    Qcursor_in_non_selected_windows = intern ("cursor-in-non-selected-windows");
    staticpro (&Qcursor_in_non_selected_windows);
+   Qalternate_cursor_type = intern ("alternate-cursor-type");
+   staticpro (&Qalternate_cursor_type);
    Qgrow_only = intern ("grow-only");
    staticpro (&Qgrow_only);
    Qinhibit_menubar_update = intern ("inhibit-menubar-update");
***************
*** 15513,15523 ****
  go back to their normal size.  */);
    Vresize_mini_windows = Qgrow_only;
  
!   DEFVAR_BOOL ("cursor-in-non-selected-windows",
! 	       &cursor_in_non_selected_windows,
!     doc: /* *Non-nil means display a hollow cursor in non-selected windows.
! nil means don't display a cursor there.  */);
!   cursor_in_non_selected_windows = 1;
  
    DEFVAR_BOOL ("auto-hscroll-mode", &automatic_hscrolling_p,
      doc: /* *Non-nil means scroll the display automatically to make point visible.  */);
--- 15523,15538 ----
  go back to their normal size.  */);
    Vresize_mini_windows = Qgrow_only;
  
!   DEFVAR_LISP ("cursor-in-non-selected-windows",
! 	       &Vcursor_in_non_selected_windows,
!     doc: /* *Cursor type to display in non-selected windows.
! t means display a hollow cursor in non-selected windows, nil means
! don't display a cursor there.  See `cursor-type' for other values.  */);
!   Vcursor_in_non_selected_windows = Qt;
! 
!   DEFVAR_LISP ("alternate-cursor-type", &Valternate_cursor_type,
!     doc: /* *Cursor type displayed in the blinking cursor off state.  */);
!   Valternate_cursor_type = Qt;
  
    DEFVAR_BOOL ("auto-hscroll-mode", &automatic_hscrolling_p,
      doc: /* *Non-nil means scroll the display automatically to make point visible.  */);
Index: xterm.c
===================================================================
RCS file: /cvs/emacs/src/xterm.c,v
retrieving revision 1.750
diff -c -r1.750 xterm.c
*** xterm.c	14 Aug 2002 00:59:01 -0000	1.750
--- xterm.c	27 Aug 2002 14:48:47 -0000
***************
*** 11808,11813 ****
--- 11808,11814 ----
       int on, hpos, vpos, x, y;
  {
    struct frame *f = XFRAME (w->frame);
+   Lisp_Object alt_cursor;
    int new_cursor_type;
    int new_cursor_width;
    int cursor_off_state = 0;
***************
*** 11849,11854 ****
--- 11850,11856 ----
       the cursor type given by the frame parameter.  If explicitly
       marked off, draw no cursor.  In all other cases, we want a hollow
       box cursor.  */
+   alt_cursor = Qalternate_cursor_type;
    new_cursor_width = -1;
    new_cursor_type = -2;
  
***************
*** 11859,11866 ****
      {
        if (w == XWINDOW (echo_area_window))
  	new_cursor_type = FRAME_DESIRED_CURSOR (f);
!       else if (NILP (Fbuffer_local_value (Qcursor_in_non_selected_windows,
! 					  w->buffer)))
  	new_cursor_type = NO_CURSOR;
        else
  	cursor_off_state = 1;
--- 11861,11868 ----
      {
        if (w == XWINDOW (echo_area_window))
  	new_cursor_type = FRAME_DESIRED_CURSOR (f);
!       else if ((alt_cursor = Fbuffer_local_value (Qcursor_in_non_selected_windows,
! 						  w->buffer), NILP (alt_cursor)))
  	new_cursor_type = NO_CURSOR;
        else
  	cursor_off_state = 1;
***************
*** 11871,11878 ****
  	   || w != XWINDOW (f->selected_window))
      {
        if ((MINI_WINDOW_P (w) && minibuf_level == 0)
! 	  || NILP (Fbuffer_local_value (Qcursor_in_non_selected_windows,
! 					w->buffer))
  	  || NILP (XBUFFER (w->buffer)->cursor_type))
  	new_cursor_type = NO_CURSOR;
        else
--- 11873,11880 ----
  	   || w != XWINDOW (f->selected_window))
      {
        if ((MINI_WINDOW_P (w) && minibuf_level == 0)
! 	  || (alt_cursor = Fbuffer_local_value (Qcursor_in_non_selected_windows,
! 						w->buffer), NILP (alt_cursor))
  	  || NILP (XBUFFER (w->buffer)->cursor_type))
  	new_cursor_type = NO_CURSOR;
        else
***************
*** 11898,11904 ****
       if it has blinked off or for nonselected windows.  */
    if (w->cursor_off_p || cursor_off_state)
      {
!       if (new_cursor_type == FILLED_BOX_CURSOR)
  	new_cursor_type = HOLLOW_BOX_CURSOR;
        else if (new_cursor_type == BAR_CURSOR && new_cursor_width > 1)
  	new_cursor_width = 1;
--- 11900,11912 ----
       if it has blinked off or for nonselected windows.  */
    if (w->cursor_off_p || cursor_off_state)
      {
!       if ((!EQ (alt_cursor, Qalternate_cursor_type) ||
! 	   (alt_cursor = Fbuffer_local_value (Qalternate_cursor_type,
! 					      w->buffer))) &&
! 	  !EQ (alt_cursor, Qt))
! 	new_cursor_type = x_specified_cursor_type (alt_cursor,
! 						   &new_cursor_width);
!       else if (new_cursor_type == FILLED_BOX_CURSOR)
  	new_cursor_type = HOLLOW_BOX_CURSOR;
        else if (new_cursor_type == BAR_CURSOR && new_cursor_width > 1)
  	new_cursor_width = 1;



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

Re: [spiegel@gnu.org: Re: [Fwd: vc-annotate causes Emacs to die]]

[Richard Stallman]

    > This is of course true, but vc-annotate still used to work prior to your
    > modification. 

    Can someone tell me what modification that is?

    What's the recipe to reproduce this?  And with which version of Emacs?

I don't know, but I am not sure it matters.  The change in VC seems to
have exposed a bug in the display code; however, the VC change itself
is surely not at fault.  What you need is a test case.

Re: [spiegel@gnu.org: Re: [Fwd: vc-annotate causes Emacs to die]]

[Richard Stallman]

I installed your change, since that is the best way to get it tested.

I changed the cursor shape handling because it seems more logical
for a cursor to blink to the nonselected-window shape.  That way,
there is a single "semi-off" shape for the cursor.  But since you
feel strongly about this, I added a variable blink-cursor-alist
that controls how the blinking works.

Re: [spiegel@gnu.org: Re: [Fwd: vc-annotate causes Emacs to die]]

[Alan Shutko]

Richard Stallman <rms@gnu.org> writes:

> I installed your change, since that is the best way to get it tested.

I see the ChangeLog entry, but it doesn't appear the file has
changed.  Have you committed it yet?

Gerd's patch does fix the problem on my system.

-- 
Alan Shutko <ats@acm.org> - In a variety of flavors!
Doin' it the hard way: Like making love while standing up in a hammock?

Re: [spiegel@gnu.org: Re: [Fwd: vc-annotate causes Emacs to die]]

[Miles Bader]

gerd.moellmann@t-online.de (Gerd Moellmann) writes:
> The cursor shape for a bar cursor in non-selected windows has become a
> thin bar cursor.  Default: 2 pixels in selected window, 1 pixel in
> other windows.

Oh, I see; I don't use a bar-cursor.

I can see some logic behind the new behavior, though -- if non-selected
windows use a hollow-box cursor even when the select-window cursor is a
bar, it seems as if it would make the non-selected windows stand out
_more_ than the select window, which seems wrong...

-Miles
-- 
Saa, shall we dance?  (from a dance-class advertisement)

Re: [spiegel@gnu.org: Re: [Fwd: vc-annotate causes Emacs to die]]

[Gerd Moellmann]

Miles Bader <miles@lsi.nec.co.jp> writes:

> gerd.moellmann@t-online.de (Gerd Moellmann) writes:
> > The cursor shape for a bar cursor in non-selected windows has become a
> > thin bar cursor.  Default: 2 pixels in selected window, 1 pixel in
> > other windows.
> 
> Oh, I see; I don't use a bar-cursor.
> 
> I can see some logic behind the new behavior, though -- if non-selected
> windows use a hollow-box cursor even when the select-window cursor is a
> bar, it seems as if it would make the non-selected windows stand out
> _more_ than the select window, which seems wrong...

The 1 pixel difference can be very hard to see; I think it's a
difference of ~0.2 mm on my monitor with a horz resolution of 1600.
Without looking close and comparing cursors, I personally can't tell
what is what.  And that's with my glasses on :).

Another issue is that other programs, notably xterm, display a hollow
box cursor when not selected.  (Some programs, like KDE editors, don't
display a cursor when not selected; I think that's a mistake.)  In any
case, if the hollow box were used consistently, it's appearance on a
window would clearly signal ``this window is not selected''.  If each
app uses its own idea of non-selected window cursor, things become
ambiguous.  I mentioned xterm, because it comes with X, and I think
it's a good move to be consistent with its UI, lacking an X style
guide.

Re: [spiegel@gnu.org: Re: [Fwd: vc-annotate causes Emacs to die]]

[Gerd Moellmann]

Richard Stallman <rms@gnu.org> writes:

> But since you feel strongly about this, I added a variable
> blink-cursor-alist that controls how the blinking works.

Thank you very much, Richard.

Your change works, but it seems to require a change of `cursor-type'
(frame parameter) after setting blink-cursor-alist, to let it take
effect.

Re: [spiegel@gnu.org: Re: [Fwd: vc-annotate causes Emacs to die]]

[Gerd Moellmann]

Richard Stallman <rms@gnu.org> writes:

>     > This is of course true, but vc-annotate still used to work prior to your
>     > modification. 
> 
>     Can someone tell me what modification that is?
> 
>     What's the recipe to reproduce this?  And with which version of Emacs?
> 
> I don't know, but I am not sure it matters.  The change in VC seems to
> have exposed a bug in the display code; however, the VC change itself
> is surely not at fault.

I find that seeing such changes often helps me start debugging with a
working hypothesis, which can shorten debugging time considerably
(that works pretty often, actually).

Re: [spiegel@gnu.org: Re: [Fwd: vc-annotate causes Emacs to die]]

[Richard Stallman]

    I see the ChangeLog entry, but it doesn't appear the file has
    changed.  Have you committed it yet?

Apparently not.  I will make sure.

Alas, there is a stuck lock, so I can't do a commit now at all.

Re: [spiegel@gnu.org: Re: [Fwd: vc-annotate causes Emacs to die]]

[Richard Stallman]

    Your change works, but it seems to require a change of `cursor-type'
    (frame parameter) after setting blink-cursor-alist, to let it take
    effect.

That is right.  It is just a variable, so nothing actually changes
when you set it.

Re: [spiegel@gnu.org: Re: [Fwd: vc-annotate causes Emacs to die]]

[Richard Stallman]

    I find that seeing such changes often helps me start debugging with a
    working hypothesis, which can shorten debugging time considerably
    (that works pretty often, actually).

If that will help you investigate, then it is a useful thing to do.
Andre, could you show Gerd the change in question?

Re: [spiegel@gnu.org: Re: [Fwd: vc-annotate causes Emacs to die]]

[Juanma Barranquero]

On Wed, 28 Aug 2002 19:32:22 -0400, Richard Stallman <rms@gnu.org> wrote:

> Alas, there is a stuck lock, so I can't do a commit now at all.

Last time it happened, there was some talk about installing a cron job
that could delete such locks after a while. Why has not been done? Lack
of time, or a technical problem of some sort with the proposed fix?


                                                           /L/e/k/t/u

Re: [spiegel@gnu.org: Re: [Fwd: vc-annotate causes Emacs to die]]

[Miles Bader]

Juanma Barranquero <lektu@terra.es> writes:
> Last time it happened, there was some talk about installing a cron job
> that could delete such locks after a while. Why has not been done? Lack
> of time, or a technical problem of some sort with the proposed fix?

Usually it seems to come down to:

  * The people who have access to the machine either don't care or
    don't have time
  * The people who care and have time don't have access to the machine
  * By the time the latter group can get access, they've either run out
    of time, or stopped caring

and of course

  * Talk is cheap

-Miles
-- 
`Life is a boundless sea of bitterness'

Re: [spiegel@gnu.org: Re: [Fwd: vc-annotate causes Emacs to die]]

[Gerd Moellmann]

Richard Stallman <rms@gnu.org> writes:

>     I find that seeing such changes often helps me start debugging with a
>     working hypothesis, which can shorten debugging time considerably
>     (that works pretty often, actually).
> 
> If that will help you investigate, then it is a useful thing to do.
> Andre, could you show Gerd the change in question?

It's no longer necessary.  Andre told me what the change was, I could
debug it from there, etc.

Re: [spiegel@gnu.org: Re: [Fwd: vc-annotate causes Emacs to die]]

[Kim F. Storm]

Richard Stallman <rms@gnu.org> writes:

>     Your change works, but it seems to require a change of `cursor-type'
>     (frame parameter) after setting blink-cursor-alist, to let it take
>     effect.
> 
> That is right.  It is just a variable, so nothing actually changes
> when you set it.

I fixed that.

If you set (buffer-local) cursor-type different from t and nil, it is
now looked up in blink-cursor-alist to blink the cursor off.

There is also a new alternate-cursor-type variable which may override
this buffer locally (or globally).

Finally, cursor-in-non-selected-windows can now be set to a specific
cursor type.

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