Re: [Emacs.app dev]: ghost cursor problem is still there

Re: [Emacs.app dev]: ghost cursor problem is still there

[David Reitter]

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

On 22 Jul 2008, at 08:39, Adrian Robert wrote:
>
> Thanks, great stuff!  But could you please summarize the changes?   
> It shows like every line was changed (maybe because of indentation?)  
> but it doesn't look like that was actually the case.  Could you  
> regenerate the patch w/o indentation/tab changes?

Okay, have a look at what's below.

The main changes are that we check cursor_type instead of cursorType  
and draw the text glyph rather than the cursor when erasing anything  
(`hl' variable).  There's a range of steps that we do to ensure that  
the cursor area is actually visible; I'm not sure if they are really  
needed, but the corresponding X code does it, too.
There is a good bit of guess-work involved, but I'm sure that testing  
will take care of any problems.

>> I don't understand why internal-show-cursor  and the w- 
>> >cursor_off_p (that it sets) do not work in the NS port - they are  
>> checked by redisplay_internal.  Is there any commentary regarding  
>> stuff like that?
>
> This is the way it is..  ;-/  Usually you just have to get your  
> hands dirty and try to trace it back -- where does w->cursor_off_p  
> get set, when/how are other terms calling or triggering functions  
> that set it or trigger calling of draw_window_cursor(), etc..

Yeah, that's how I've been working... it doesn't really feel like  
2008. (And not even like 1999.)

> Did you have any sort of a start on using the emacs common code  
> (even though it didn't work)?  Maybe if you post a patch for that  
> others could take a stab at moving it further.

Well, if someone could explain why redisplay noticed a change in - 
 >cursor_off_p in Carbon or X, but not in NS, then that would already  
be helpful.  I don't think enabling the original blink-cursor-mode  
will be difficult, and you get a lot of functionality for free that way.

I'm trying again with a few extra TRACE steps enabled.

cc'ing emacs-devel for some advice regarding the patch.

- D



===================================================================
RCS file: /sources/emacs/emacs/src/nsterm.m,v
retrieving revision 1.11
diff -c -w -r1.11 nsterm.m
*** nsterm.m	21 Jul 2008 17:47:24 -0000	1.11
--- nsterm.m	22 Jul 2008 13:30:21 -0000
***************
*** 2274,2279 ****
--- 2274,2285 ----
                          int on_p, int active_p)
   /*  
--------------------------------------------------------------------------
        External call (RIF): draw cursor
+      (modeled after x_draw_window_cursor and erase_phys_cursor.
+      FIXME: erase_phys_cursor is called from display_and_set_cursor,
+      called from update_window_cursor/x_update_window_end/...
+      Why do we have to duplicate this code?
+      Also, why doesn't cursor_off_p (internal_show_cursor) work?
+      This prevents the original blink-cursor-mode from working.
       
-------------------------------------------------------------------------- */
   {
     NSRect r, s;
***************
*** 2282,2295 ****
     struct glyph *phys_cursor_glyph;
     int overspill;
     unsigned char drawGlyph = 0, cursorType, oldCursorType;

     NSTRACE (dumpcursor);

!   if (!on_p)
         return;

     w->phys_cursor_type = cursor_type;
!   w->phys_cursor_on_p = 1;

     if (cursor_type == NO_CURSOR)
       {
--- 2288,2310 ----
     struct glyph *phys_cursor_glyph;
     int overspill;
     unsigned char drawGlyph = 0, cursorType, oldCursorType;
+   int new_cursor_type;
+   int new_cursor_width;
+   int active_cursor;
+   enum draw_glyphs_face hl;
+   struct glyph_matrix *active_glyphs = w->current_matrix;
+   Display_Info *dpyinfo = FRAME_X_DISPLAY_INFO (f);
+   int hpos = w->phys_cursor.hpos;
+   int vpos = w->phys_cursor.vpos;
+   struct glyph_row *cursor_row;

     NSTRACE (dumpcursor);

!   if (!on_p) // check this?    && !w->phys_cursor_on_p)
         return;

     w->phys_cursor_type = cursor_type;
!   w->phys_cursor_on_p = on_p;

     if (cursor_type == NO_CURSOR)
       {
***************
*** 2322,2333 ****
--- 2337,2350 ----
     if (overspill > 0)
       r.size.width -= overspill;

+
     /* PENDING: 23: use emacs stored f->cursor_type instead of ns- 
specific */
     oldCursorType = FRAME_CURSOR (f);
     cursorType = FRAME_CURSOR (f) = FRAME_NEW_CURSOR (f);
     f->output_data.ns->current_cursor_color
       = f->output_data.ns->desired_cursor_color;

+
     /* PENDING: only needed in rare cases with last-resort font in  
HELLO..
        should we do this more efficiently? */
     ns_clip_to_row (w, glyph_row, -1, NULL);
***************
*** 2346,2357 ****
     if (cursorType == no_highlight || cursor_type == NO_CURSOR)
       {
         /* clearing for blink: erase the cursor itself */
         [FRAME_BACKGROUND_COLOR (f) set];
!       cursorType = oldCursorType; /* just clear what we had before */
       }
     else
         [FRAME_CURSOR_COLOR (f) set];

     if (!active_p)
       {
         /* inactive window: ignore what we just set and use a hollow  
box */
--- 2363,2441 ----
     if (cursorType == no_highlight || cursor_type == NO_CURSOR)
       {
         /* clearing for blink: erase the cursor itself */
+
+       /* No cursor displayed or row invalidated => nothing to do on  
the
+ 	 screen.  */
+       if (w->phys_cursor_type == NO_CURSOR)
+ 	return;
+
+       /* VPOS >= active_glyphs->nrows means that window has been  
resized.
+ 	 Don't bother to erase the cursor.  */
+       if (vpos >= active_glyphs->nrows)
+ 	return;
+
+       /* If row containing cursor is marked invalid, there is  
nothing we
+ 	 can do.  */
+       cursor_row = MATRIX_ROW (active_glyphs, vpos);
+       if (!cursor_row->enabled_p)
+ 	return;
+
+       /* If line spacing is > 0, old cursor may only be partially  
visible in
+ 	 window after split-window.  So adjust visible height.  */
+       cursor_row->visible_height = min (cursor_row->visible_height,
+ 					window_text_bottom_y (w) - cursor_row->y);
+
+       /* If row is completely invisible, don't attempt to delete a  
cursor which
+ 	 isn't there.  This can happen if cursor is at top of a window, and
+ 	 we switch to a buffer with a header line in that window.  */
+       if (cursor_row->visible_height <= 0)
+ 	return;
+
+       /* If cursor is in the fringe, erase by drawing actual bitmap  
there.  */
+       if (cursor_row->cursor_in_fringe_p)
+ 	{
+ 	  cursor_row->cursor_in_fringe_p = 0;
+ 	  draw_fringe_bitmap (w, cursor_row, 0);
+ 	  return;
+ 	}
+
+       /* This can happen when the new row is shorter than the old one.
+ 	 In this case, either draw_glyphs or clear_end_of_line
+ 	 should have cleared the cursor.  Note that we wouldn't be
+ 	 able to erase the cursor in this case because we don't have a
+ 	 cursor glyph at hand.  */
+       if (w->phys_cursor.hpos >= cursor_row->used[TEXT_AREA])
+ 	return;
+
+       /* If the cursor is in the mouse face area, redisplay that when
+ 	 we clear the cursor.  */
+       if (! NILP (dpyinfo->mouse_face_window)
+ 	  && w == XWINDOW (dpyinfo->mouse_face_window)
+ 	  && (vpos > dpyinfo->mouse_face_beg_row
+ 	      || (vpos == dpyinfo->mouse_face_beg_row
+ 		  && hpos >= dpyinfo->mouse_face_beg_col))
+ 	  && (vpos < dpyinfo->mouse_face_end_row
+ 	      || (vpos == dpyinfo->mouse_face_end_row
+ 		  && hpos < dpyinfo->mouse_face_end_col))
+ 	  /* Don't redraw the cursor's spot in mouse face if it is at the
+ 	     end of a line (on a newline).  The cursor appears there, but
+ 	     mouse highlighting does not.  */
+ 	  && cursor_row->used[TEXT_AREA] > hpos)
+ 	hl = DRAW_MOUSE_FACE;
+       else
+ 	hl = DRAW_NORMAL_TEXT;
+       drawGlyph = 1; // just draw the Glyph
         [FRAME_BACKGROUND_COLOR (f) set];
!
!       NSDisableScreenUpdates ();
       }
     else
+     {
+       cursorType = cursor_type;
+       hl = DRAW_CURSOR;
         [FRAME_CURSOR_COLOR (f) set];

+
         if (!active_p)
   	{
   	  /* inactive window: ignore what we just set and use a hollow box  
*/
***************
*** 2359,2396 ****
         [FRAME_CURSOR_COLOR (f) set];
       }

     switch (cursorType)
       {
!     case no_highlight:
         break;
!     case filled_box:
         NSRectFill (r);
         drawGlyph = 1;
         break;
!     case hollow_box:
         NSRectFill (r);
         [FRAME_BACKGROUND_COLOR (f) set];
         NSRectFill (NSInsetRect (r, 1, 1));
         [FRAME_CURSOR_COLOR (f) set];
         drawGlyph = 1;
         break;
!     case underscore:
         s = r;
         s.origin.y += lrint (0.75 * s.size.height);
!       s.size.height = lrint (s.size.height * 0.25);
         NSRectFill (s);
         break;
!     case bar:
         s = r;
!       s.size.width = 1;
         NSRectFill (s);
         break;
       }
     ns_unfocus (f);

     /* if needed, draw the character under the cursor */
     if (drawGlyph)
!     draw_phys_cursor_glyph (w, glyph_row, DRAW_CURSOR);
   }


--- 2443,2487 ----
   	  [FRAME_CURSOR_COLOR (f) set];
   	}

+       NSDisableScreenUpdates ();
+
         switch (cursorType)
   	{
! 	case NO_CURSOR: // no_highlight:
   	  break;
! 	case FILLED_BOX_CURSOR: //filled_box:
   	  NSRectFill (r);
   	  drawGlyph = 1;
   	  break;
! 	case HOLLOW_BOX_CURSOR: //hollow_box:
   	  NSRectFill (r);
   	  [FRAME_BACKGROUND_COLOR (f) set];
   	  NSRectFill (NSInsetRect (r, 1, 1));
   	  [FRAME_CURSOR_COLOR (f) set];
   	  drawGlyph = 1;
   	  break;
! 	case HBAR_CURSOR: // underscore:
   	  s = r;
   	  s.origin.y += lrint (0.75 * s.size.height);
! 	  s.size.height = cursor_width; //lrint (s.size.height * 0.25);
   	  NSRectFill (s);
   	  break;
! 	case BAR_CURSOR: //bar:
   	  s = r;
! 	  s.size.width = cursor_width;
   	  NSRectFill (s);
+ 	  drawGlyph = 1;
   	  break;
   	}
+     }
     ns_unfocus (f);

     /* if needed, draw the character under the cursor */
     if (drawGlyph)
!     draw_phys_cursor_glyph (w, glyph_row, hl);
!
!   NSEnableScreenUpdates ();
!
   }


~/Projects/Aquamacs/emacs/src$


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 2193 bytes --]

Re: [Emacs.app dev]: ghost cursor problem is still there

[Adrian Robert]

On Jul 22, 2008, at 9:41 AM, David Reitter wrote:

> Well, if someone could explain why redisplay noticed a change in - 
> >cursor_off_p in Carbon or X, but not in NS, then that would already  
> be helpful.  I don't think enabling the original blink-cursor-mode  
> will be difficult, and you get a lot of functionality for free that  
> way.

I think this might be due to differences in event loop handling  
between NS and the other ports.  I've summarized the NS port approach  
to event loop integration and a few other areas in nextstep/DEV-NOTES.

Basically, the NS port does not poll for events, but goes into a loop  
where only a timeout firing or a user event will return control to  
emacs.  (Sometimes the timeout period is specified by emacs itself.)   
I'm guessing that maybe waiting for events is supposed to be  
interrupted (for a redisplay pass) when emacs internally decides to  
change the cursor state, but this interrupting is not happening for  
some reason.

Re: [Emacs.app dev]: ghost cursor problem is still there

[David Reitter]

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

Adrian et al,

On 22 Jul 2008, at 09:41, David Reitter wrote:
>
> The main changes are that we check cursor_type instead of cursorType  
> and draw the text glyph rather than the cursor when erasing anything  
> (`hl' variable).  There's a range of steps that we do to ensure that  
> the cursor area is actually visible; I'm not sure if they are really  
> needed, but the corresponding X code does it, too.
> There is a good bit of guess-work involved, but I'm sure that  
> testing will take care of any problems.

OK, in my latest build, things work a lot better and the original  
blink-cursor-mode appears to work fine now.  (I don't quite understand  
which change made the event mechanisms work better.)

To repeat, these changes address the following issues:

- frame background rather than the right glyph in the white-out phases  
during blinking
- `cursor-type' variable as in core Emacs, rather than NS specific  
solution
- with it, support for things like (box . 2)
- box/hollow cursors too narrow
- standard blink-cursor-mode with all its bells and whistles (whether  
one needs them or not)

I would also take the blink rate stuff out of the preferences (a patch  
to the nib) - it doesn't work with the generic blinking code and I  
believe it's there for historic reasons (because the NS port  
implemented blinking separately) rather than because it would be very  
important to have for users (one could think of much more important  
settings that could be there).

ns_set_cursor_type is now equivalent to the x_set_cursor_type (quasi)  
generic.  I left ns_set_cursor_color for you or someone else.

The occasional ghost cursors seem to remain.

Any objections?

D



Index: nsterm.m
===================================================================
RCS file: /sources/emacs/emacs/src/nsterm.m,v
retrieving revision 1.23
diff -c -r1.23 nsterm.m
*** nsterm.m	5 Aug 2008 03:05:14 -0000	1.23
--- nsterm.m	20 Aug 2008 05:18:41 -0000
***************
*** 168,180 ****
      the Function modifer (laptops).  May be any of the modifier lisp  
symbols. */
   Lisp_Object ns_function_modifier;

- /* A floating point value specifying the rate at which to blink the  
cursor.
-    YES indicates 0.5, NO indicates no blinking. */
- Lisp_Object ns_cursor_blink_rate;
-
- /* Used for liason with core emacs cursor-blink-mode. */
- Lisp_Object ns_cursor_blink_mode;
-
   /* A floating point value specifying vertical stretch (positive) or  
shrink
     (negative) of text line spacing.  Zero means default spacing.
     YES indicates 0.5, NO indicates 0.0. */
--- 168,173 ----
***************
*** 235,241 ****
   static NSEvent *last_appdefined_event = 0;
   static NSTimer *timed_entry = 0;
   static NSTimer *fd_entry = nil;
- static NSTimer *cursor_blink_entry = nil;
   static NSTimer *scroll_repeat_entry = nil;
   static fd_set select_readfds, t_readfds;
   static struct timeval select_timeout;
--- 228,233 ----
***************
*** 2270,2275 ****
--- 2262,2271 ----
                          int on_p, int active_p)
   /*  
--------------------------------------------------------------------------
        External call (RIF): draw cursor
+      (modeled after x_draw_window_cursor and erase_phys_cursor.
+      FIXME: erase_phys_cursor is called from display_and_set_cursor,
+      called from update_window_cursor/x_update_window_end/...
+      Why do we have to duplicate this code?
       
-------------------------------------------------------------------------- */
   {
     NSRect r, s;
***************
*** 2278,2291 ****
     struct glyph *phys_cursor_glyph;
     int overspill;
     unsigned char drawGlyph = 0, cursorType, oldCursorType;

     NSTRACE (dumpcursor);

!   if (!on_p)
         return;

     w->phys_cursor_type = cursor_type;
!   w->phys_cursor_on_p = 1;

     if (cursor_type == NO_CURSOR)
       {
--- 2274,2296 ----
     struct glyph *phys_cursor_glyph;
     int overspill;
     unsigned char drawGlyph = 0, cursorType, oldCursorType;
+   int new_cursor_type;
+   int new_cursor_width;
+   int active_cursor;
+   enum draw_glyphs_face hl;
+   struct glyph_matrix *active_glyphs = w->current_matrix;
+   Display_Info *dpyinfo = FRAME_X_DISPLAY_INFO (f);
+   int hpos = w->phys_cursor.hpos;
+   int vpos = w->phys_cursor.vpos;
+   struct glyph_row *cursor_row;

     NSTRACE (dumpcursor);

!   if (!on_p) // check this?    && !w->phys_cursor_on_p)
         return;

     w->phys_cursor_type = cursor_type;
!   w->phys_cursor_on_p = on_p;

     if (cursor_type == NO_CURSOR)
       {
***************
*** 2318,2326 ****
     if (overspill > 0)
       r.size.width -= overspill;

-   /* TODO: 23: use emacs stored f->cursor_type instead of ns- 
specific */
     oldCursorType = FRAME_CURSOR (f);
     cursorType = FRAME_CURSOR (f) = FRAME_NEW_CURSOR (f);
     f->output_data.ns->current_cursor_color
       = f->output_data.ns->desired_cursor_color;

--- 2323,2332 ----
     if (overspill > 0)
       r.size.width -= overspill;

     oldCursorType = FRAME_CURSOR (f);
     cursorType = FRAME_CURSOR (f) = FRAME_NEW_CURSOR (f);
+
+   /* TODO: 23: use emacs stored cursor color instead of ns-specific */
     f->output_data.ns->current_cursor_color
       = f->output_data.ns->desired_cursor_color;

***************
*** 2342,2392 ****
     if (cursorType == no_highlight || cursor_type == NO_CURSOR)
       {
         /* clearing for blink: erase the cursor itself */
         [FRAME_BACKGROUND_COLOR (f) set];
!       cursorType = oldCursorType; /* just clear what we had before */
       }
     else
         [FRAME_CURSOR_COLOR (f) set];

!   if (!active_p)
!     {
!       /* inactive window: ignore what we just set and use a hollow  
box */
!       cursorType = hollow_box;
!       [FRAME_CURSOR_COLOR (f) set];
!     }

!   switch (cursorType)
!     {
!     case no_highlight:
!       break;
!     case filled_box:
!       NSRectFill (r);
!       drawGlyph = 1;
!       break;
!     case hollow_box:
!       NSRectFill (r);
!       [FRAME_BACKGROUND_COLOR (f) set];
!       NSRectFill (NSInsetRect (r, 1, 1));
!       [FRAME_CURSOR_COLOR (f) set];
!       drawGlyph = 1;
!       break;
!     case underscore:
!       s = r;
!       s.origin.y += lrint (0.75 * s.size.height);
!       s.size.height = lrint (s.size.height * 0.25);
!       NSRectFill (s);
!       break;
!     case bar:
!       s = r;
!       s.size.width = 1;
!       NSRectFill (s);
!       break;
       }
     ns_unfocus (f);

     /* if needed, draw the character under the cursor */
     if (drawGlyph)
!     draw_phys_cursor_glyph (w, glyph_row, DRAW_CURSOR);
   }


--- 2348,2472 ----
     if (cursorType == no_highlight || cursor_type == NO_CURSOR)
       {
         /* clearing for blink: erase the cursor itself */
+
+       /* No cursor displayed or row invalidated => nothing to do on  
the
+ 	 screen.  */
+       if (w->phys_cursor_type == NO_CURSOR)
+ 	return;
+
+       /* VPOS >= active_glyphs->nrows means that window has been  
resized.
+ 	 Don't bother to erase the cursor.  */
+       if (vpos >= active_glyphs->nrows)
+ 	return;
+
+       /* If row containing cursor is marked invalid, there is  
nothing we
+ 	 can do.  */
+       cursor_row = MATRIX_ROW (active_glyphs, vpos);
+       if (!cursor_row->enabled_p)
+ 	return;
+
+       /* If line spacing is > 0, old cursor may only be partially  
visible in
+ 	 window after split-window.  So adjust visible height.  */
+       cursor_row->visible_height = min (cursor_row->visible_height,
+ 					window_text_bottom_y (w) - cursor_row->y);
+
+       /* If row is completely invisible, don't attempt to delete a  
cursor which
+ 	 isn't there.  This can happen if cursor is at top of a window, and
+ 	 we switch to a buffer with a header line in that window.  */
+       if (cursor_row->visible_height <= 0)
+ 	return;
+
+       /* If cursor is in the fringe, erase by drawing actual bitmap  
there.  */
+       if (cursor_row->cursor_in_fringe_p)
+ 	{
+ 	  cursor_row->cursor_in_fringe_p = 0;
+ 	  draw_fringe_bitmap (w, cursor_row, 0);
+ 	  return;
+ 	}
+
+       /* This can happen when the new row is shorter than the old one.
+ 	 In this case, either draw_glyphs or clear_end_of_line
+ 	 should have cleared the cursor.  Note that we wouldn't be
+ 	 able to erase the cursor in this case because we don't have a
+ 	 cursor glyph at hand.  */
+       if (w->phys_cursor.hpos >= cursor_row->used[TEXT_AREA])
+ 	return;
+
+       /* If the cursor is in the mouse face area, redisplay that when
+ 	 we clear the cursor.  */
+       if (! NILP (dpyinfo->mouse_face_window)
+ 	  && w == XWINDOW (dpyinfo->mouse_face_window)
+ 	  && (vpos > dpyinfo->mouse_face_beg_row
+ 	      || (vpos == dpyinfo->mouse_face_beg_row
+ 		  && hpos >= dpyinfo->mouse_face_beg_col))
+ 	  && (vpos < dpyinfo->mouse_face_end_row
+ 	      || (vpos == dpyinfo->mouse_face_end_row
+ 		  && hpos < dpyinfo->mouse_face_end_col))
+ 	  /* Don't redraw the cursor's spot in mouse face if it is at the
+ 	     end of a line (on a newline).  The cursor appears there, but
+ 	     mouse highlighting does not.  */
+ 	  && cursor_row->used[TEXT_AREA] > hpos)
+ 	hl = DRAW_MOUSE_FACE;
+       else
+ 	hl = DRAW_NORMAL_TEXT;
+       drawGlyph = 1; // just draw the Glyph
         [FRAME_BACKGROUND_COLOR (f) set];
!
!       NSDisableScreenUpdates ();
       }
     else
+     {
+       cursorType = cursor_type;
+       hl = DRAW_CURSOR;
         [FRAME_CURSOR_COLOR (f) set];
+

!       if (!active_p)
! 	{
! 	  /* inactive window: ignore what we just set and use a hollow box */
! 	  cursorType = hollow_box;
! 	  [FRAME_CURSOR_COLOR (f) set];
! 	}

!       NSDisableScreenUpdates ();
!
!       switch (cursorType)
! 	{
! 	case NO_CURSOR: // no_highlight:
! 	  break;
! 	case FILLED_BOX_CURSOR: //filled_box:
! 	  NSRectFill (r);
! 	  drawGlyph = 1;
! 	  break;
! 	case HOLLOW_BOX_CURSOR: //hollow_box:
! 	  NSRectFill (r);
! 	  [FRAME_BACKGROUND_COLOR (f) set];
! 	  NSRectFill (NSInsetRect (r, 1, 1));
! 	  [FRAME_CURSOR_COLOR (f) set];
! 	  drawGlyph = 1;
! 	  break;
! 	case HBAR_CURSOR: // underscore:
! 	  s = r;
! 	  s.origin.y += lrint (0.75 * s.size.height);
! 	  s.size.height = cursor_width; //lrint (s.size.height * 0.25);
! 	  NSRectFill (s);
! 	  break;
! 	case BAR_CURSOR: //bar:
! 	  s = r;
! 	  s.size.width = cursor_width;
! 	  NSRectFill (s);
! 	  drawGlyph = 1;
! 	  break;
! 	}
       }
     ns_unfocus (f);

     /* if needed, draw the character under the cursor */
     if (drawGlyph)
!     draw_phys_cursor_glyph (w, glyph_row, hl);
!
!   NSEnableScreenUpdates ();
!
   }


***************
*** 3173,3207 ****
                                                 repeats: YES]
                  retain];

-   if (!NILP (ns_cursor_blink_mode) && !cursor_blink_entry)
-     {
-       if (!NUMBERP (ns_cursor_blink_rate))
-         ns_cursor_blink_rate = make_float (0.5);
-       cursor_blink_entry = [[NSTimer
-         scheduledTimerWithTimeInterval: XFLOATINT  
(ns_cursor_blink_rate)
-                                 target: NSApp
-                               selector: @selector  
(cursor_blink_handler:)
-                               userInfo: 0
-                                repeats: YES]
-                              retain];
-     }
-   else if (NILP (ns_cursor_blink_mode) && cursor_blink_entry)
-     {
-       if (NUMBERP (ns_cursor_blink_rate))
-           ns_cursor_blink_rate = Qnil;
-       struct ns_display_info *dpyinfo = x_display_list; /* HACK */
-       [cursor_blink_entry invalidate];
-       [cursor_blink_entry release];
-       cursor_blink_entry = 0;
-       if (dpyinfo->x_highlight_frame)
-         {
-           Lisp_Object tem
- 	    = get_frame_param (dpyinfo->x_highlight_frame, Qcursor_type);
-           dpyinfo->x_highlight_frame->output_data.ns->desired_cursor
- 	    = ns_lisp_to_cursor_type (tem);
-         }
-     }
-
     /* Let Application dispatch events until it receives an event of  
the type
          NX_APPDEFINED, which should only be sent by  
timeout_handler.  */
     inNsSelect = 1;
--- 3253,3258 ----
***************
*** 3487,3494 ****
     ns_command_modifier = Qsuper;
     ns_control_modifier = Qcontrol;
     ns_function_modifier = Qnone;
-   ns_cursor_blink_rate = Qnil;
-   ns_cursor_blink_mode = Qnil;
     ns_expand_space = make_float (0.0);
     ns_antialias_text = Qt;
     ns_antialias_threshold = 10.0; /* not exposed to lisp side */
--- 3538,3543 ----
***************
*** 3795,3804 ****
                Qnil, Qnil, NO, YES);
     if (NILP (ns_function_modifier))
       ns_function_modifier = Qnone;
-   ns_default ("CursorBlinkRate", &ns_cursor_blink_rate,
-              make_float (0.5), Qnil, YES, NO);
-   if (NUMBERP (ns_cursor_blink_rate))
-     ns_cursor_blink_mode = Qt;
     ns_default ("ExpandSpace", &ns_expand_space,
                make_float (0.5), make_float (0.0), YES, NO);
     ns_default ("GSFontAntiAlias", &ns_antialias_text,
--- 3844,3849 ----
***************
*** 4194,4224 ****

   extern void update_window_cursor (struct window *w, int on);

- - (void)cursor_blink_handler: (NSTimer *)cursorEntry
- /*  
--------------------------------------------------------------------------
-      Flash the cursor
-     
-------------------------------------------------------------------------- */
- {
-   struct ns_display_info *dpyinfo = x_display_list; /*HACK, but OK  
for now */
-   struct frame *f = dpyinfo->x_highlight_frame;
-   NSTRACE (cursor_blink_handler);
-
-   if (!f)
-     return;
-   if (f->output_data.ns->current_cursor == no_highlight)
-     {
-       Lisp_Object tem = get_frame_param (f, Qcursor_type);
-       f->output_data.ns->desired_cursor = ns_lisp_to_cursor_type  
(tem);
-     }
-   else
-     {
-       f->output_data.ns->desired_cursor = no_highlight;
-     }
-   update_window_cursor (XWINDOW (FRAME_SELECTED_WINDOW (f)), 1);
-   /*x_update_cursor (f, 1); */
- }
-
-
   - (void)fd_handler: (NSTimer *) fdEntry
   /*  
--------------------------------------------------------------------------
        Check data waiting on file descriptors and terminate if so
--- 4239,4244 ----
***************
*** 6025,6039 ****
     int cursorType
       = ns_lisp_to_cursor_type (get_frame_param (frame, Qcursor_type));
     prevExpandSpace = XFLOATINT (ns_expand_space);
-   prevBlinkRate = NILP (ns_cursor_blink_rate)
-     ? 0 : XFLOATINT (ns_cursor_blink_rate);

   #ifdef NS_IMPL_COCOA
     prevUseHighlightColor = ns_use_system_highlight_color;
   #endif

     [expandSpaceSlider setFloatValue: prevExpandSpace];
-   [cursorBlinkSlider setFloatValue: prevBlinkRate];
     [cursorTypeMatrix selectCellWithTag: (cursorType == filled_box ?  
1 :
                                           (cursorType == bar ? 2 :
                                            (cursorType ==  
underscore ? 3 : 4)))];
--- 6045,6056 ----
***************
*** 6062,6070 ****
     int ctrlTag = [[controlModMenu selectedItem] tag];
     int fnTag = [[functionModMenu selectedItem] tag];
   #endif
-   float blinkRate = [cursorBlinkSlider floatValue];
     float expandSpace = [expandSpaceSlider floatValue];
-   Lisp_Object old_cursor_blink_mode;

     if (expandSpace != prevExpandSpace)
       {
--- 6079,6085 ----
***************
*** 6075,6112 ****
              x_set_window_size (frame, 0, frame->text_cols, frame- 
 >text_lines); */
         prevExpandSpace = expandSpace;
       }
-   if (blinkRate != prevBlinkRate)
-     {
-       old_cursor_blink_mode = ns_cursor_blink_mode;
-       if (blinkRate == 0.0)
-         {
-           ns_cursor_blink_rate = Qnil;
-           ns_cursor_blink_mode = Qnil;
-         }
-       else
-         {
-           ns_cursor_blink_rate = make_float (blinkRate);
-           ns_cursor_blink_mode = Qt;
-         }
-       if (!EQ (ns_cursor_blink_mode, old_cursor_blink_mode))
-           Feval (Fcons (intern ("blink-cursor-mode"), Qnil));
-
-       if (blinkRate != 0.0 && prevBlinkRate != 0.0)
-         {  /* if changed rates, remove blink handler so change  
picked up */
-           struct ns_display_info *dpyinfo = FRAME_NS_DISPLAY_INFO  
(frame);
-           [cursor_blink_entry invalidate];
-           [cursor_blink_entry release];
-           cursor_blink_entry = 0;
-           if (dpyinfo->x_highlight_frame)
-             {
-               Lisp_Object tem
- 		= get_frame_param (dpyinfo->x_highlight_frame, Qcursor_type);
-               dpyinfo->x_highlight_frame->output_data.ns- 
 >desired_cursor
- 		= ns_lisp_to_cursor_type (tem);
-             }
-         }
-       prevBlinkRate = blinkRate;
-     }
     FRAME_NEW_CURSOR (frame)
       = (cursorTag == 1 ? filled_box
          : cursorTag == 2 ? bar
--- 6090,6095 ----
***************
*** 6419,6432 ****
   Set to none means that the function key is not interpreted by Emacs  
at all,\n\
   allowing it to be used at a lower level for accented character  
entry.");

-   DEFVAR_LISP ("ns-cursor-blink-rate", &ns_cursor_blink_rate,
-                "Rate at which the Emacs cursor blinks (in seconds).\n\
- Set to nil to disable blinking.");
-
-   DEFVAR_LISP ("ns-cursor-blink-mode", &ns_cursor_blink_mode,
-                "Internal variable -- use M-x blink-cursor-mode or  
preferences\n\
- panel to control this setting.");
-
     DEFVAR_LISP ("ns-expand-space", &ns_expand_space,
                  "Amount by which spacing between lines is expanded  
(positive)\n\
   or shrunk (negative).  Zero (the default) means standard line  
height.\n\
--- 6402,6407 ----

*** nsfns.m	09 Aug 2008 04:37:53 -0400	1.20
--- nsfns.m	19 Aug 2008 13:36:53 -0400	
***************
*** 412,417 ****
--- 412,418 ----
       }
   }

+ /* FIXME: adapt to generics */

   static void
   ns_set_cursor_color (struct frame *f, Lisp_Object arg, Lisp_Object  
oldval)
***************
*** 435,440 ****
--- 436,453 ----
     update_face_from_frame_parameter (f, Qcursor_color, arg);
   }

+ /* this is like x_set_cursor_type defined in xfns.c */
+ void
+ ns_set_cursor_type (f, arg, oldval)
+      FRAME_PTR f;
+      Lisp_Object arg, oldval;
+ {
+   set_frame_cursor_types (f, arg);
+
+   /* Make sure the cursor gets redrawn.  */
+   cursor_type_changed = 1;
+ }
+ \f

   static void
   ns_set_icon_name (struct frame *f, Lisp_Object arg, Lisp_Object  
oldval)
***************
*** 929,954 ****
   }


- static void
- ns_set_cursor_type (struct frame *f, Lisp_Object arg, Lisp_Object  
oldval)
- {
-   int val;
-
-   val = ns_lisp_to_cursor_type (arg);
-   if (val >= 0)
-     {
-       f->output_data.ns->desired_cursor =val;
-     }
-   else
-     {
-       store_frame_param (f, Qcursor_type, oldval);
-       error ("the `cursor-type' frame parameter should be either  
`no', `box', \
- `hollow', `underscore' or `bar'.");
-     }
-
-   update_mode_lines++;
- }
-

   /* 23: called to set mouse pointer color, but all other terms use  
it to
          initialize pointer types (and don't set the color ;) */
--- 942,947 ----


*** ns-win.el	18 Aug 2008 12:23:42 -0400	1.24
--- ns-win.el	19 Aug 2008 12:42:35 -0400	
***************
*** 59,65 ****
   ;; nsterm.m
   (defvar ns-version-string)
   (defvar ns-expand-space)
- (defvar ns-cursor-blink-rate)
   (defvar ns-alternate-modifier)

   ;;;; Command line argument handling.
--- 59,64 ----
***************
*** 995,1004 ****
     (ns-set-resource nil "CommandModifier" (symbol-name ns-command- 
modifier))
     (ns-set-resource nil "ControlModifier" (symbol-name ns-control- 
modifier))
     (ns-set-resource nil "FunctionModifier" (symbol-name ns-function- 
modifier))
-   (ns-set-resource nil "CursorBlinkRate"
-                    (if ns-cursor-blink-rate
-                        (number-to-string ns-cursor-blink-rate)
-                      "NO"))
     (ns-set-resource nil "ExpandSpace"
                      (if ns-expand-space
                          (number-to-string ns-expand-space)
--- 994,999 ----
***************
*** 1228,1257 ****
   				   0 1)) ))
     (if (not tool-bar-mode) (tool-bar-mode t)))

- (defvar ns-cursor-blink-mode) 		; nsterm.m
-
- ;; Redefine from frame.el.
- (define-minor-mode blink-cursor-mode
-   "Toggle blinking cursor mode.
- With a numeric argument, turn blinking cursor mode on if ARG is  
positive,
- otherwise turn it off.  When blinking cursor mode is enabled, the
- cursor of the selected window blinks.
-
- Note that this command is effective only when Emacs
- displays through a window system, because then Emacs does its own
- cursor display.  On a text-only terminal, this is not implemented."
-   :init-value (not (or noninteractive
- 		       no-blinking-cursor
- 		       (eq ns-cursor-blink-rate nil)))
-   :initialize 'custom-initialize-safe-default
-   :group 'cursor
-   :global t
-   (if blink-cursor-mode
-       (setq ns-cursor-blink-mode t)
-       (setq ns-cursor-blink-mode nil)))
-
-
-
   ;;;; Dialog-related functions.

   ;; Ask user for confirm before printing.  Due to Kevin Rodgers.
--- 1223,1228 ----



[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 2193 bytes --]

Re: [Emacs.app dev]: ghost cursor problem is still there

[Adrian Robert]

On Aug 20, 2008, at 1:22 AM, David Reitter wrote:

> Adrian et al,
>
> On 22 Jul 2008, at 09:41, David Reitter wrote:
>>
>> The main changes are that we check cursor_type instead of  
>> cursorType and draw the text glyph rather than the cursor when  
>> erasing anything (`hl' variable).  There's a range of steps that we  
>> do to ensure that the cursor area is actually visible; I'm not sure  
>> if they are really needed, but the corresponding X code does it, too.
>> There is a good bit of guess-work involved, but I'm sure that  
>> testing will take care of any problems.
>
> OK, in my latest build, things work a lot better and the original  
> blink-cursor-mode appears to work fine now.  (I don't quite  
> understand which change made the event mechanisms work better.)

This is great news.


> To repeat, these changes address the following issues:
>
> - frame background rather than the right glyph in the white-out  
> phases during blinking
> - `cursor-type' variable as in core Emacs, rather than NS specific  
> solution
> - with it, support for things like (box . 2)
> - box/hollow cursors too narrow

OK, these were from before.


> - standard blink-cursor-mode with all its bells and whistles  
> (whether one needs them or not)

This is new, and good.


> I would also take the blink rate stuff out of the preferences (a  
> patch to the nib) - it doesn't work with the generic blinking code  
> and I believe it's there for historic reasons (because the NS port  
> implemented blinking separately) rather than because it would be  
> very important to have for users (one could think of much more  
> important settings that could be there).

This is a matter of opinion and at least I personally find blink and  
other cursor attributes I like to change easily and frequently.  It  
should be an extremely simple matter to update the prefs controller  
code to set the core emacs blink rate, and I will do so once you've  
checked your changes in.  The prefs window may be removed completely  
before 23.1 as it faces hostility from many quarters so I'd rather  
keep it fully functional until then.


> The occasional ghost cursors seem to remain.

I've been occasionally looking at this when I've had the chance.  It  
has not been easy to track down.  I get it (Leopard) in latest source  
and for a while back to around the merge, but not in rc2a.  I think  
the best chance will be to try to find the first date/version when the  
problem appeared and then diff the source code (xdisp.c, dispnew.c,  
and xterm.c will probably tell the story).  If anyone has information  
on this, please post it.


> Any objections?

I'm fine with this being committed.


thanks,
Adrian

Re: [Emacs.app dev]: ghost cursor problem is still there

[David Reitter]

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

On 20 Aug 2008, at 08:27, Adrian Robert wrote:

> I'm fine with this being committed.

done.

- D

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 2193 bytes --]

Re: [Emacs.app dev]: ghost cursor problem is still there

[Nick Roberts]

 > > I'm fine with this being committed.
 > 
 > done.

There were problems with this patch, e.g., ns-cursor-blink-rate not defined
which appears to have been corrected but I still get grey strips which are
opaque to the text until the cursor moves over them or I select the region
with the mouse.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: [Emacs.app dev]: ghost cursor problem is still there

[David Reitter]

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

On 21 Aug 2008, at 03:33, Nick Roberts wrote:
>
> There were problems with this patch, e.g., ns-cursor-blink-rate not  
> defined
> which appears to have been corrected

Thanks Adrian for catching the missing checkin.

> but I still get grey strips which are
> opaque to the text until the cursor moves over them or I select the  
> region
> with the mouse.

Can you reproduce starting with Emacs -Q?
This may depend on your cursor type settings.
I can't reproduce this with Cocoa.

Or are we talking about the old ghost cursor problem:?

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 2193 bytes --]

Re: [Emacs.app dev]: ghost cursor problem is still there

[Adrian Robert]

On Aug 21, 2008, at 5:08 AM, David Reitter wrote:

> On 21 Aug 2008, at 03:33, Nick Roberts wrote:
>>
>> There were problems with this patch, e.g., ns-cursor-blink-rate not  
>> defined
>> which appears to have been corrected
>
> Thanks Adrian for catching the missing checkin.
>
>> but I still get grey strips which are
>> opaque to the text until the cursor moves over them or I select the  
>> region
>> with the mouse.
>
> Can you reproduce starting with Emacs -Q?

I can. After startup, select a row of text, click onto another  
application, then back.  I don't know if this was there before the  
patch.

As far as the (possibly separate) ghost cursor problem, I narrowed  
down that it appeared between rc2a and rc3.  I'm studying the source  
code diffs to determine the problem.


Adrian

Re: [Emacs.app dev]: ghost cursor problem is still there

[Adrian Robert]

On Aug 21, 2008, at 8:07 AM, Adrian Robert wrote:

>
> On Aug 21, 2008, at 5:08 AM, David Reitter wrote:
>
>> On 21 Aug 2008, at 03:33, Nick Roberts wrote:
>>>
>>> but I still get grey strips which are
>>> opaque to the text until the cursor moves over them or I select  
>>> the region
>>> with the mouse.
>>
>> Can you reproduce starting with Emacs -Q?
>
> I can. After startup, select a row of text, click onto another  
> application, then back.  I don't know if this was there before the  
> patch.

I checked, it was not.  In addition the patch introduces the following  
bug: the cursor is not changed to hollow box from block when focus on  
the frame is taken away.  And when multiple frames are present, things  
get very messy.  If these problems cannot be resolved quickly we  
should probably revert the patch, or at least the non blink-related  
parts of it.

Re: [Emacs.app dev]: ghost cursor problem is still there

[David Reitter]

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

On 21 Aug 2008, at 08:07, Adrian Robert wrote:
>>>
>>> but I still get grey strips which are
>>> opaque to the text until the cursor moves over them or I select  
>>> the region
>>> with the mouse.
>>
>> Can you reproduce starting with Emacs -Q?
>
> I can. After startup, select a row of text, click onto another  
> application, then back.  I don't know if this was there before the  
> patch.

It appears that -Q is not respected: org.gnu.Emacs.plist is still read.
That's why I couldn't reproduce this, but I can see the bars now.

This only occurs when cursor-type is set to t.

D

PS.: Getting these.  Presumably unrelated.

2008-08-21 11:03:37.251 Emacs[13746:10b] Unlocking Focus on wrong view  
(<EmacsView: 0x2bbc510>), expected <NSView: 0x2bc1900>
2008-08-21 11:03:38.385 Emacs[13746:10b] Unlocking Focus on wrong view  
(<EmacsView: 0x2bbc510>), expected <NSView: 0x2bc1900>


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 2193 bytes --]

Re: [Emacs.app dev]: ghost cursor problem is still there

[David Reitter]

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

On 21 Aug 2008, at 09:16, Adrian Robert wrote:
>>
>> I can. After startup, select a row of text, click onto another  
>> application, then back.  I don't know if this was there before the  
>> patch.
>
> I checked, it was not.  In addition the patch introduces the  
> following bug: the cursor is not changed to hollow box from block  
> when focus on the frame is taken away.

This turned out to be the same, simple to fix bug.

I have checked in a fix (the essential bit is below).  Nick, let me  
know how that's working for you.



***************
*** 2424,2431 ****
         if (!active_p)
   	{
   	  /* inactive window: ignore what we just set and use a hollow box  
*/
! 	  cursorType = hollow_box;
! 	  [FRAME_CURSOR_COLOR (f) set];
   	}

         NSDisableScreenUpdates ();
--- 2426,2432 ----
         if (!active_p)
   	{
   	  /* inactive window: ignore what we just set and use a hollow box  
*/
! 	  cursorType = HOLLOW_BOX_CURSOR;
   	}

         NSDisableScreenUpdates ();

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 2193 bytes --]

Re: [Emacs.app dev]: ghost cursor problem is still there

[Adrian Robert]

On Aug 21, 2008, at 11:51 AM, David Reitter wrote:

> On 21 Aug 2008, at 09:16, Adrian Robert wrote:
>>>
>>> I can. After startup, select a row of text, click onto another  
>>> application, then back.  I don't know if this was there before the  
>>> patch.
>>
>> I checked, it was not.  In addition the patch introduces the  
>> following bug: the cursor is not changed to hollow box from block  
>> when focus on the frame is taken away.
>
> This turned out to be the same, simple to fix bug.

OK, this fixes the bug for switching between two frames.  However w/ 
only one frame active, clicking to another app does not switch to a  
hollow box.  In addition, a ghost cursor can be generated if the  
cursor is on some text, then you click at the bottom of the buffer  
after focus is given.  (But the same recipe fails for multiple frames,  
even when switching to another app instead of the other frame.)  I  
believe these are enough clues to indicate the ghost cursor problem  
relates to a suite of focus-handling changes in core emacs that came  
in between rc2a and rc3, but were never fully sync'ed up with from the  
NS side.  Seiji Zenitani also mentioned something about this a while  
back.

I will try to allocate a block of time to look at it this this  
weekend, but if someone else can take a look / a crack at it before  
then, I (and I'm sure other users) would be grateful.  The relevant  
functions (maybe best to look at w32term.c, not xterm) are  
x_new_focus_frame, x_detect_focus_change et al., while under NS there  
are currently -windowDidBecomeKey, -windowDidResignKey.


-Adrian

Re: [Emacs.app dev]: ghost cursor problem is still there

[Nick Roberts]

 > I have checked in a fix (the essential bit is below).  Nick, let me  
 > know how that's working for you.

It might not be the full fix, as Adrian suggests, but it works for my
usage - thanks.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: [Emacs.app dev]: ghost cursor problem is still there

[Dan Nicolaescu]

David Reitter <david.reitter@gmail.com> writes:

  > Adrian et al,
  > 
  > On 22 Jul 2008, at 09:41, David Reitter wrote:
  > >
  > > The main changes are that we check cursor_type instead of cursorType  
  > > and draw the text glyph rather than the cursor when erasing anything  
  > > (`hl' variable).  There's a range of steps that we do to ensure that  
  > > the cursor area is actually visible; I'm not sure if they are really  
  > > needed, but the corresponding X code does it, too.
  > > There is a good bit of guess-work involved, but I'm sure that  
  > > testing will take care of any problems.
  > 
  > OK, in my latest build, things work a lot better and the original  
  > blink-cursor-mode appears to work fine now.  (I don't quite understand  
  > which change made the event mechanisms work better.)
  > 
  > To repeat, these changes address the following issues:
  > 
  > - frame background rather than the right glyph in the white-out phases  
  > during blinking
  > - `cursor-type' variable as in core Emacs, rather than NS specific  
  > solution
  > - with it, support for things like (box . 2)
  > - box/hollow cursors too narrow
  > - standard blink-cursor-mode with all its bells and whistles (whether  
  > one needs them or not)
  > 
  > I would also take the blink rate stuff out of the preferences (a patch  
  > to the nib) - it doesn't work with the generic blinking code and I  
  > believe it's there for historic reasons (because the NS port  
  > implemented blinking separately) rather than because it would be very  
  > important to have for users (one could think of much more important  
  > settings that could be there).
  > 
  > ns_set_cursor_type is now equivalent to the x_set_cursor_type (quasi)  
  > generic.  I left ns_set_cursor_color for you or someone else.
  > 
  > The occasional ghost cursors seem to remain.
  > 
  > Any objections?

[snip]

  > +       drawGlyph = 1; // just draw the Glyph
  >          [FRAME_BACKGROUND_COLOR (f) set];
  > !
  > !       NSDisableScreenUpdates ();


Unfortunately this breaks GNUStep:

gcc -rdynamic `./prefix-args -Xlinker -z nocombreloc -L/usr/lib -L/usr/lib -lgnustep-gui -lgnustep-base -lobjc  -lpthread` -o temacs dispnew.o frame.o scroll.o xdisp.o menu.o  window.o charset.o coding.o category.o ccl.o character.o chartab.o cm.o term.o terminal.o xfaces.o    emacs.o keyboard.o macros.o keymap.o sysdep.o buffer.o filelock.o insdel.o marker.o minibuf.o fileio.o dired.o filemode.o cmds.o casetab.o casefiddle.o indent.o search.o regex.o undo.o alloc.o data.o doc.o editfns.o callint.o eval.o floatfns.o fns.o font.o print.o lread.o syntax.o unexelf.o bytecode.o process.o callproc.o region-cache.o sound.o atimer.o doprnt.o strftime.o intervals.o textprop.o composite.o md5.o  nsterm.o nsfns.o nsmenu.o nsselect.o nsimage.o nsfont.o fontset.o fringe.o image.o   terminfo.o lastfile
 .o   vm-limit.o         -lncurses   -lm 
nsterm.o: In function `ns_draw_window_cursor':
/tmp/emacs/src/nsterm.m:2432: undefined reference to `NSDisableScreenUpdates'
/tmp/emacs/src/nsterm.m:2417: undefined reference to `NSDisableScreenUpdates'
/tmp/emacs/src/nsterm.m:2469: undefined reference to `NSEnableScreenUpdates'
/tmp/emacs/src/nsterm.m:2432: undefined reference to `NSDisableScreenUpdates'
collect2: ld returned 1 exit status
make[1]: *** [temacs] Error 1

Also please make sure that your log entries are properly formatted:

----------------------------
revision 1.24
date: 2008-08-20 19:46:37 +0200;  author: davidswelt;  state: Exp;  lines: +125 -150;  commitid: c97vNELzQ5S6kyft;
Clear cursor properly rather than redrawing the area. Respect width of
bar cursors. remove ns-specific code for cursor blinking.
----------------------------

Like ChangeLog, it should contain the changed function name, 

Re: [Emacs.app dev]: ghost cursor problem is still there

[David Reitter]

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

On 23 Aug 2008, at 13:30, Dan Nicolaescu wrote:
>>
>> +       drawGlyph = 1; // just draw the Glyph
>>         [FRAME_BACKGROUND_COLOR (f) set];
>> !
>> !       NSDisableScreenUpdates ();
>
>
> Unfortunately this breaks GNUStep:

Not a problem; I have checked in a fix.
Can you let us know how well this works on GNUStep?
Is there a lot of flicker when moving the cursor or in blink-cursor- 
mode?

>  Also please make sure that your log entries are properly formatted:
>
> Like ChangeLog, it should contain the changed function name,

OK, I can do that.  I modeled my earlier comments after others in that  
file, e.g. from 2008-08-01.

Could you explain to me how the ChangeLog and the CVS comments should  
differ semantically?

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 2193 bytes --]

Re: [Emacs.app dev]: ghost cursor problem is still there

[Dan Nicolaescu]

["emacs.app dev list" <emacs-app-dev-@lists.sourceforge.net>
 trimmed from the CC list, please do not CC closed list to posts to
 public mailing lists].

David Reitter <david.reitter@gmail.com> writes:

  > On 23 Aug 2008, at 13:30, Dan Nicolaescu wrote:
  > >>
  > >> +       drawGlyph = 1; // just draw the Glyph
  > >>         [FRAME_BACKGROUND_COLOR (f) set];
  > >> !
  > >> !       NSDisableScreenUpdates ();
  > >
  > >
  > > Unfortunately this breaks GNUStep:
  > 
  > Not a problem; I have checked in a fix.

Thanks.

  > Can you let us know how well this works on GNUStep?
  > Is there a lot of flicker when moving the cursor or in blink-cursor-
  > mode?

Unfortunately, the only machine I have access to that runs GNUStep is
behind a slow link, so it won't be relevant...

  > >  Also please make sure that your log entries are properly formatted:
  > >
  > > Like ChangeLog, it should contain the changed function name,
  > 
  > OK, I can do that.  I modeled my earlier comments after others in that
  > file, e.g. from 2008-08-01.
  > 
  > Could you explain to me how the ChangeLog and the CVS comments should
  > differ semantically?

AFAIK they should be identical, except for formatting, no need to indent
the CVS logs with a TAB.
If you use PCL-CVS to do the checkin, and first write the ChangeLog
entry, it will automatically copy and reformat it into the log buffer.

Re: [Emacs.app dev]: ghost cursor problem is still there

[David Reitter]

On 23 Aug 2008, at 17:13, Dan Nicolaescu wrote:

> AFAIK they should be identical, except for formatting, no need to  
> indent
> the CVS logs with a TAB.

Right, and the CVS comment for the ChangeLog is then supposed to  
contain a third copy of the same info - is that correct?

Re: [Emacs.app dev]: ghost cursor problem is still there

[Dan Nicolaescu]

David Reitter <david.reitter@gmail.com> writes:

  > On 23 Aug 2008, at 17:13, Dan Nicolaescu wrote:
  > 
  > > AFAIK they should be identical, except for formatting, no need to
  > > indent
  > > the CVS logs with a TAB.
  > 
  > Right, and the CVS comment for the ChangeLog is then supposed to
  > contain a third copy of the same info - is that correct?

AFAIK there's no strong rule about that one.
My personal prefererence to check in both the ChangeLog and the change
itself at the same time, so implicitly they will both have the same log
entry, but many people don't do it that way.
I'd guess the explanation is that nobody wants to look at the log for
ChangeLog.

Re: [Emacs.app dev]: ghost cursor problem is still there

[Nick Roberts]

 > Right, and the CVS comment for the ChangeLog is then supposed to  
 > contain a third copy of the same info - is that correct?

The ChangeLog doesn't need a log message, two copies are enough: one per file;
and one in the ChangeLog as a poor man's changeset information.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: [Emacs.app dev]: ghost cursor problem is still there

[Dan Nicolaescu]

David Reitter <david.reitter@gmail.com> writes:

  > On 23 Aug 2008, at 13:30, Dan Nicolaescu wrote:
  > >>
  > >> +       drawGlyph = 1; // just draw the Glyph
  > >>         [FRAME_BACKGROUND_COLOR (f) set];
  > >> !
  > >> !       NSDisableScreenUpdates ();
  > >
  > >
  > > Unfortunately this breaks GNUStep:
  > 
  > Not a problem; I have checked in a fix.

Bad things still happen:


[snip]
Generating autoloads for kermit.el...
Generating autoloads for kermit.el...done
Generating autoloads for kmacro.el...
Generating autoloads for kmacro.el...done
Fatal error (11)Aborted
make[2]: *** [autoloads] Error 134
make[2]: Leaving directory `/tmp/emacs/lisp'
make[1]: *** [/tmp/emacs/src/../lisp/loaddefs.el] Error 2
make[1]: Leaving directory `/tmp/emacs/src'
make: *** [src] Error 2

It worked on August 19th.  No idea if this is in any way related to your
change...

Re: [Emacs.app dev]: ghost cursor problem is still there

[Andreas Schwab]

Nick Roberts <nickrob@snap.net.nz> writes:

>  > Right, and the CVS comment for the ChangeLog is then supposed to  
>  > contain a third copy of the same info - is that correct?
>
> The ChangeLog doesn't need a log message, two copies are enough: one per file;
> and one in the ChangeLog as a poor man's changeset information.

If you commit everything in a single change set then there are only two
copies.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [Emacs.app dev]: ghost cursor problem is still there

[David Reitter]

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

On 23 Aug 2008, at 21:53, Dan Nicolaescu wrote:
>
> [snip]
> Generating autoloads for kermit.el...
> Generating autoloads for kermit.el...done
> Generating autoloads for kmacro.el...
> Generating autoloads for kmacro.el...done
> Fatal error (11)Aborted
> make[2]: *** [autoloads] Error 134
> make[2]: Leaving directory `/tmp/emacs/lisp'
> make[1]: *** [/tmp/emacs/src/../lisp/loaddefs.el] Error 2
> make[1]: Leaving directory `/tmp/emacs/src'
> make: *** [src] Error 2
>
> It worked on August 19th.  No idea if this is in any way related to  
> your
> change...

I don't see how.  Any earlier error messages in that log?
Did you 'make clean'?

Is it possible to test stuff using GNUStep on a Fedora Core system (to  
which I have user-level access)?


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 2193 bytes --]

Re: [Emacs.app dev]: ghost cursor problem is still there

[Dan Nicolaescu]

David Reitter <david.reitter@gmail.com> writes:

  > On 23 Aug 2008, at 21:53, Dan Nicolaescu wrote:
  > >
  > > [snip]
  > > Generating autoloads for kermit.el...
  > > Generating autoloads for kermit.el...done
  > > Generating autoloads for kmacro.el...
  > > Generating autoloads for kmacro.el...done
  > > Fatal error (11)Aborted
  > > make[2]: *** [autoloads] Error 134
  > > make[2]: Leaving directory `/tmp/emacs/lisp'
  > > make[1]: *** [/tmp/emacs/src/../lisp/loaddefs.el] Error 2
  > > make[1]: Leaving directory `/tmp/emacs/src'
  > > make: *** [src] Error 2
  > >
  > > It worked on August 19th.  No idea if this is in any way related to
  > > your
  > > change...
  > 
  > I don't see how.  Any earlier error messages in that log?

Nothing interesting.

  > Did you 'make clean'?

Sure.

  > Is it possible to test stuff using GNUStep on a Fedora Core system
  > (to which I have user-level access)?

You can probably compile it from source.  I looked for binaries for
Fedora at some point, but I couldn't find any.