display bug for empty lines in recent git snapshot

display bug for empty lines in recent git snapshot

[Giorgos Keramidas]

I've compiled a new snapshot of Emacs after a few weeks from the git
mirror at git://repo.or.cz/emacs.git and noticed that there's now a
display bug with empty lines.

The latest snapshot I compiled was last afternoon, from:

    commit 30286b8e4b37619c8d6bf5bc01840606f26d9cf7
    Author: Adrian Robert <Adrian.B.Robert@gmail.com>
    Date:   Tue Oct 26 16:20:00 2010 +0300

        * term/ns-win.el (ns-new-frame, ns-show-prefs): Don't add to global
          map.
        * term/common-win.el (x-setup-function-keys): Remove most of the
          keymappings.  Comment on the remaining ones.

When a file contains only the following:

    > cat -vte foo.txt
    foo$
    $
    bar$
    >

Moving the point to the beginning of the second line displays the
following in my emacs -nw sessions:

    ,-----------------------------------------------------------
    |foo
    | #
    |bar
    |
    |-UU-:**--F1  foo.txt        All (2,0)      (Text) -----
    |
    `-----------------------------------------------------------

Emacs obviously thinks that point is at (2, 0) but moves the terminal
cursor to the _second_ column instead of the first, at the location of
the '#' character in the output shown above.

This only happens for empty lines, so I'm guessing it's something
related to the calculation of line length?  I will try to go backwards
in the commit history of the last 1-2 months to see when this first
started happening, but in the meantime I thought I should report this
here too.

This is all tested with...

In GNU Emacs 24.0.50.1 (x86_64-unknown-linux-gnu, GTK+ Version 2.20.1)
 of 2010-11-16 on gkeramidas-glaptop
configured using `configure  '--prefix=/opt/emacs' '--infodir=/opt/emacs/info''

Important settings:
  value of $LC_ALL: el_GR.utf8
  value of $LC_COLLATE: nil
  value of $LC_CTYPE: nil
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: el_GR.utf8
  value of $XMODIFIERS: nil
  locale-coding-system: utf-8-unix
  default enable-multibyte-characters: t

Major mode: Text

Minor modes in effect:
  iswitchb-mode: t
  cua-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
ESC [ > 8 3 ; 4 0 0 0 3 ; 0 c f o o RET RET b a r RET
ESC O A ESC O A ESC x r e p o r t - b u TAB RET

Recent messages:
("/opt/emacs/bin/emacs" "foo.txt")
Ido mode disabled
Loading vc...done
For information about GNU Emacs and the GNU system, type <f1> C-a.
(New file)
Loading vc-git...done
Loading vc-hg...done
Auto-saving...done

Load-path shadows:
None found.

Features:
(shadow sort gnus-util mail-extr message sendmail rfc822 mml easymenu
mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231 rfc2047 rfc2045
ietf-drums mm-util mail-prsvr mailabbrev mail-utils gmm-utils mailheader
warnings emacsbug vc-hg vc-git .emacs jka-compr keramida-slime
keramida-defaults vc vc-dispatcher uniquify advice help-fns
advice-preload ido paren iswitchb cua-base hl-line keramida-hooks
keramida-autoload keramida-lang keramida-keys edmacro kmacro keramida-ui
bbdb regexp-opt timezone disp-table tooltip ediff-hook vc-hooks
lisp-float-type mwheel x-win x-dnd tool-bar dnd fontset image fringe
lisp-mode register page menu-bar rfn-eshadow timer select scroll-bar
mouse jit-lock font-lock syntax facemenu font-core frame cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese case-table epa-hook jka-cmpr-hook help simple abbrev
loaddefs button minibuffer faces cus-face files text-properties overlay
md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote make-network-process dbusbind
dynamic-setting system-font-setting font-render-setting move-toolbar gtk
x-toolkit x multi-tty emacs)

Re: display bug for empty lines in recent git snapshot

[Tassilo Horn]

Hi Giorgos,

I've already filed this as bug#7417.

Bye,
Tassilo

Re: display bug for empty lines in recent git snapshot

[Giorgos Keramidas]

On Wed, 17 Nov 2010 10:31:08 +0100, Tassilo Horn <tassilo@member.fsf.org> wrote:
> Hi Giorgos,
>
> I've already filed this as bug#7417.

Thanks!  I'm almost done bisecting through the history of trunk, so I
will have a better idea of when it started in 5-10 minutes.  I'll try to
update the bug with any information I find.

Re: display bug for empty lines in recent git snapshot

[Giorgos Keramidas]

On Wed, 17 Nov 2010 10:31:08 +0100, Tassilo Horn <tassilo@member.fsf.org> wrote:
> Hi Giorgos,
>
> I've already filed this as bug#7417.

The change that introduced this bug in my local mirror of the git trunk
is one committed by Eli on Nov 13:

: changeset:   111519:3355ca38548d
: user:        Eli Zaretskii <eliz@gnu.org>
: date:        Sat Nov 13 15:29:31 2010 +0200
: files:       doc/emacs/ChangeLog doc/emacs/display.texi doc/emacs/mule.texi
:              doc/emacs/rmail.texi doc/lispref/ChangeLog
:              doc/lispref/display.texi etc/ChangeLog etc/NEWS lisp/ChangeLog
:              lisp/international/characters.el src/.gdbinit src/ChangeLog
:              src/dispextern.h src/term.c src/w32term.c src/xdisp.c src/xterm.c
: description:
: Fix and document display of glyphless characters.
:
:  src/xdisp.c (set_cursor_from_row): Fix cursor positioning on
:  zero-width characters.
:  (syms_of_xdisp) <glyphless-char-display>: Doc fix.
:  src/.gdbinit (pgx): Adapt to latest changes in `struct glyph'.
:  src/w32term.c (x_draw_glyphless_glyph_string_foreground): Draw the
:  box before drawing the glyphs inside it.
:  src/dispextern.h (enum glyphless_display_method): Rename
:  GLYPHLESS_DISPLAY_HEXA_CODE to GLYPHLESS_DISPLAY_HEX_CODE.  All
:  users changed.
:  src/term.c (append_glyphless_glyph, produce_glyphless_glyph): Fix
:  comments.
:  (produce_glyphless_glyph): Enclose "U+nnnn" and "empty box"
:  whitespace in "[]", to simulate a box.  Don't use uninitialized
:  variable `width'.
:  lisp/international/characters.el (glyphless-char-display-control):
:  Renamed from glyphless-char-control; all users changed.  Doc fix.
:  Signal an error if display method is not one of the recognized
:  symbols.
:  doc/emacs/rmail.texi (Rmail Coding): Characters with no fonts are not
:  necessarily displayed as empty boxes.
:  doc/emacs/mule.texi (Language Environments, Fontsets): Characters with no
:  fonts are not necessarily displayed as empty boxes.
:  doc/emacs/display.texi (Text Display): Document display of glyphless
:  characters.
:  doc/lispref/display.texi (Usual Display): Characters with no fonts are not
:  necessarily displayed as empty boxes.
:  etc/NEWS: Document display of glyphless characters.

I think there's some exception we have to add here for newline
characters, because they are treated as GLYPHLESS_DISPLAY_THIN_SPACE or
as GLYPHLESS_DISPLAY_EMPTY_BOX with a width of 1 space, but I am not
sure what the best way to do this would be.  Maybe Eli has a good idea
of how this can be fixed?

Re: display bug for empty lines in recent git snapshot

[Eli Zaretskii]

> From: Giorgos Keramidas <keramida@ceid.upatras.gr>
> Cc: Tassilo Horn <tassilo@member.fsf.org>, emacs-devel@gnu.org
> Date: Wed, 17 Nov 2010 11:43:23 +0100
> 
> On Wed, 17 Nov 2010 10:31:08 +0100, Tassilo Horn <tassilo@member.fsf.org> wrote:
> > Hi Giorgos,
> >
> > I've already filed this as bug#7417.
> 
> The change that introduced this bug in my local mirror of the git trunk
> is one committed by Eli on Nov 13:

Yes, it's my bad, sorry.  Cursor positioning is a tricksy business,
with lots of different use-cases.  I guess I failed to test the TTY
display properly after this change.

> I think there's some exception we have to add here for newline
> characters, because they are treated as GLYPHLESS_DISPLAY_THIN_SPACE or
> as GLYPHLESS_DISPLAY_EMPTY_BOX with a width of 1 space, but I am not
> sure what the best way to do this would be.  Maybe Eli has a good idea
> of how this can be fixed?

No, this isn't the problem, I'm quite sure.

The problem is that I installed a fix for displaying glyphless
characters using the zero-width method (see the change in
set_cursor_from_row in the changeset), and that fix inadvertently
introduced this bug.

I will try to fix this as soon as possible.

Re: display bug for empty lines in recent git snapshot

[Giorgos Keramidas]

On Wed, 17 Nov 2010 07:54:23 -0500, Eli Zaretskii <eliz@gnu.org> wrote:
>> I think there's some exception we have to add here for newline
>> characters, because they are treated as GLYPHLESS_DISPLAY_THIN_SPACE or
>> as GLYPHLESS_DISPLAY_EMPTY_BOX with a width of 1 space, but I am not
>> sure what the best way to do this would be.  Maybe Eli has a good idea
>> of how this can be fixed?
>
> No, this isn't the problem, I'm quite sure.
>
> The problem is that I installed a fix for displaying glyphless
> characters using the zero-width method (see the change in
> set_cursor_from_row in the changeset), and that fix inadvertently
> introduced this bug.
>
> I will try to fix this as soon as possible.

That's ok.  I daily build new snapshots, and this is a very minor detail
that doesn't have any serious consequences to the actual *data* of the
buffer.  When you have a fix in the works, please feel free to forward
me the patch for testing.

Re: display bug for empty lines in recent git snapshot

[Eli Zaretskii]

> From: Giorgos Keramidas <keramida@ceid.upatras.gr>
> Cc: tassilo@member.fsf.org, emacs-devel@gnu.org
> Date: Wed, 17 Nov 2010 14:14:54 +0100
> 
> > The problem is that I installed a fix for displaying glyphless
> > characters using the zero-width method (see the change in
> > set_cursor_from_row in the changeset), and that fix inadvertently
> > introduced this bug.
> >
> > I will try to fix this as soon as possible.
> 
> That's ok.  I daily build new snapshots, and this is a very minor detail
> that doesn't have any serious consequences to the actual *data* of the
> buffer.  When you have a fix in the works, please feel free to forward
> me the patch for testing.

I think I fixed this now, patch below.

=== modified file 'src/ChangeLog'
--- src/ChangeLog	2010-11-17 15:12:02 +0000
+++ src/ChangeLog	2010-11-17 19:15:24 +0000
@@ -1,3 +1,8 @@
+2010-11-17  Eli Zaretskii  <eliz@gnu.org>
+
+	* xdisp.c (set_cursor_from_row): Fix cursor positioning in empty
+	lines on text-mode terminals.  (bug#7417)
+
 2010-11-17  Stefan Monnier  <monnier@iro.umontreal.ca>
 
 	* xterm.c (get_current_wm_state): Rename from get_current_vm_state.

=== modified file 'src/xdisp.c'
--- src/xdisp.c	2010-11-17 02:37:45 +0000
+++ src/xdisp.c	2010-11-17 19:15:24 +0000
@@ -12839,6 +12839,15 @@ set_cursor_from_row (struct window *w, s
 	&& BUFFERP (glyph->object) && glyph->charpos == pt_old)
       && bpos_covered < pt_old)
     {
+      /* An empty line has a single glyph whose OBJECT is zero and
+	 whose CHARPOS is the position of a newline on that line.
+	 Note that on a TTY, there are more glyphs after that, which
+	 were produced by extend_face_to_end_of_line, but their
+	 CHARPOS is zero or negative.  */
+      int empty_line_p =
+	(row->reversed_p ? glyph > glyphs_end : glyph < glyphs_end)
+	&& INTEGERP (glyph->object) && glyph->charpos > 0;
+
       if (row->ends_in_ellipsis_p && pos_after == last_pos)
 	{
 	  EMACS_INT ellipsis_pos;
@@ -12875,6 +12884,7 @@ set_cursor_from_row (struct window *w, s
 	       || (row->truncated_on_right_p && pt_old > bpos_max)
 	       /* Zero-width characters produce no glyphs.  */
 	       || (!string_seen
+		   && !empty_line_p
 		   && (row->reversed_p
 		       ? glyph_after > glyphs_end
 		       : glyph_after < glyphs_end)))

Re: display bug for empty lines in recent git snapshot

[Giorgos Keramidas]

Excellent!  This fixes the bug here.

Thank you :)

On Wed, 17 Nov 2010 21:20:22 +0200, Eli Zaretskii <eliz@gnu.org> wrote:
>> That's ok.  I daily build new snapshots, and this is a very minor detail
>> that doesn't have any serious consequences to the actual *data* of the
>> buffer.  When you have a fix in the works, please feel free to forward
>> me the patch for testing.
>
> I think I fixed this now, patch below.
>
> === modified file 'src/ChangeLog'
> --- src/ChangeLog	2010-11-17 15:12:02 +0000
> +++ src/ChangeLog	2010-11-17 19:15:24 +0000
> @@ -1,3 +1,8 @@
> +2010-11-17  Eli Zaretskii  <eliz@gnu.org>
> +
> +	* xdisp.c (set_cursor_from_row): Fix cursor positioning in empty
> +	lines on text-mode terminals.  (bug#7417)
> +
>  2010-11-17  Stefan Monnier  <monnier@iro.umontreal.ca>
>
>  	* xterm.c (get_current_wm_state): Rename from get_current_vm_state.
>
> === modified file 'src/xdisp.c'
> --- src/xdisp.c	2010-11-17 02:37:45 +0000
> +++ src/xdisp.c	2010-11-17 19:15:24 +0000
> @@ -12839,6 +12839,15 @@ set_cursor_from_row (struct window *w, s
>  	&& BUFFERP (glyph->object) && glyph->charpos == pt_old)
>        && bpos_covered < pt_old)
>      {
> +      /* An empty line has a single glyph whose OBJECT is zero and
> +	 whose CHARPOS is the position of a newline on that line.
> +	 Note that on a TTY, there are more glyphs after that, which
> +	 were produced by extend_face_to_end_of_line, but their
> +	 CHARPOS is zero or negative.  */
> +      int empty_line_p =
> +	(row->reversed_p ? glyph > glyphs_end : glyph < glyphs_end)
> +	&& INTEGERP (glyph->object) && glyph->charpos > 0;
> +
>        if (row->ends_in_ellipsis_p && pos_after == last_pos)
>  	{
>  	  EMACS_INT ellipsis_pos;
> @@ -12875,6 +12884,7 @@ set_cursor_from_row (struct window *w, s
>  	       || (row->truncated_on_right_p && pt_old > bpos_max)
>  	       /* Zero-width characters produce no glyphs.  */
>  	       || (!string_seen
> +		   && !empty_line_p
>  		   && (row->reversed_p
>  		       ? glyph_after > glyphs_end
>  		       : glyph_after < glyphs_end)))