bug#30226: Fixing it->pixel_width / it->current_x when tabs and line numbers.

[Keith David Bershatsky <esq@lawlist.com>]

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

Both the current_x _and_ the hpos do not advance in the simulation immediately prior to generating the tab STRETCH; whereas, the opposite true when it is the real deal with display_line.  It may very well be the case that maybe_produce_line_number is not called from move_it_in_display_line_to in some case, whereas it is called from display_line.

I set up a couple of STDERR messages in display_line to observe the values for current_x and hpos immediately prior to calling PRODUCE_GLYPHS -- with one STDERR message before calling maybe_produce_line_number, and another STDERR message immediately after calling maybe_produce_line_number.

I set up a STDERR message in x_produce_glyphs to observe the values for current_x and hpos immediately prior to the calculations for the tab STRETCH.

When we iterate over the line containing "[TAB][TAB]Hello-world" when w->hscroll == 2, moving by increments of it.pixel_width using move_it_in_display_line_to, we observe the following behavior:

* Prior to calling PRODUCE_GLYPHS for the _first_ TAB:

** it.current_x == 7.

** it.hpos == 0.

When we observe the STDERR messages produced from display_line, we observe the following behavior:

* Prior to calling PRODUCE_GLYPHS for the _first_ TAB:

** it.current_x == 35.

** it.hpos == 3.

The difference between the simulation and the real deal is directly related to a call in display_line to maybe_produce_line_number.  I have not yet figured out exactly where in the simulation we need to advance it.current_x and it.hpos so that they are in sync with the real deal (display_line).  All of the calculations beginning at xdisp.c:28247 that relate to the tab STRETCH are dependent upon it.current_x -- when that value is wrong, everything to the right also gets skewed when iterating over that current line using move_it_in_display_line_to.

	      int tab_width = it->tab_width * font->space_width;
	      int x = it->current_x + it->continuation_lines_width;
	      int x0 = x;
	      /* Adjust for line numbers, if needed.   */
	      if (!NILP (Vdisplay_line_numbers) && x0 >= it->lnum_pixel_width)
		x -= it->lnum_pixel_width;
	      int next_tab_x = ((1 + x + tab_width - 1) / tab_width) * tab_width;

	      /* If the distance from the current position to the next tab
		 stop is less than a space character width, use the
		 tab stop after that.  */
	      if (next_tab_x - x < font->space_width)
		next_tab_x += tab_width;
	      if (!NILP (Vdisplay_line_numbers) && x0 >= it->lnum_pixel_width)
		next_tab_x += (it->lnum_pixel_width
			       - ((it->w->hscroll * font->space_width)
				  % tab_width));

	      it->pixel_width = next_tab_x - x0;

Here is an excerpt of the STDERR printout using the function bug_30226 (when w->hscroll == 2) on the line containing "[TAB][TAB]Hello-world" with the following Elisp and the attached patch.diff:

    (setq display-line-numbers t)
    (setq buffer-display-table (make-display-table))
    (aset buffer-display-table
          ?\t
          (vector (make-glyph-code ?\u00BB 'font-lock-warning-face)
                  (make-glyph-code ?\t 'highlight)))
    (setq tab-width 8)
    (global-set-key [f5] (lambda () (interactive) (bug-30226)))
    (insert "\n\n\t\tHello-world")

1.  NOTHING
    it.c (0)
    w->hscroll (2)
    it.hpos (0) 
    it.current_x (0)
    it.pixel_width (0)

2.  TAB CHARACTER
    it.c (187)
    w->hscroll (2)
    it.hpos (0) 
    it.current_x (0)
    it.pixel_width (7)

x_produce_glyphs:
    it->c (9)
    hscroll (2)
    it->first_visible_x (14)
    it->current_x (7)
    it->hpos (0)
    it->lnum_pixel_width (28)

3.  TAB STRETCH
    it.c (9)
    w->hscroll (2)
    it.hpos (0) 
    it.current_x (7)
    it.pixel_width (49)

x_produce_glyphs:
    it->c (9)
    hscroll (2)
    it->first_visible_x (14)
    it->current_x (7)
    it->hpos (0)
    it->lnum_pixel_width (28)

4.  TAB CHARACTER
    it.c (187)
    w->hscroll (2)
    it.hpos (5) 
    it.current_x (84)
    it.pixel_width (7)

x_produce_glyphs:
    it->c (9)
    hscroll (2)
    it->first_visible_x (14)
    it->current_x (91)
    it->hpos (6)
    it->lnum_pixel_width (28)

5.  TAB STRETCH
    it.c (9)
    w->hscroll (2)
    it.hpos (6) 
    it.current_x (91)
    it.pixel_width (35)

x_produce_glyphs:
    it->c (9)
    hscroll (2)
    it->first_visible_x (14)
    it->current_x (91)
    it->hpos (6)
    it->lnum_pixel_width (28)

6.  TEXT
    it.c (72)
    w->hscroll (2)
    it.hpos (7) 
    it.current_x (126)
    it.pixel_width (7)

* * *

15.  TEXT
    it.c (108)
    w->hscroll (2)
    it.hpos (16) 
    it.current_x (189)
    it.pixel_width (7)

16.  TEXT
    it.c (100)
    w->hscroll (2)
    it.hpos (17) 
    it.current_x (196)
    it.pixel_width (7)

x_produce_glyphs:
    it->c (9)
    hscroll (2)
    it->first_visible_x (14)
    it->current_x (7)
    it->hpos (0)
    it->lnum_pixel_width (28)

display_line -- BEFORE maybe_produce_line_number:
    it->current_x (7)
    it->hpos (0)
    it->c (9)

display_line -- AFTER maybe_produce_line_number:
    it->current_x (35)
    it->hpos (3)
    it->c (9)

x_produce_glyphs:
    it->c (9)
    hscroll (2)
    it->first_visible_x (14)
    it->current_x (35)
    it->hpos (3)
    it->lnum_pixel_width (28)

x_produce_glyphs:
    it->c (9)
    hscroll (2)
    it->first_visible_x (14)
    it->current_x (77)
    it->hpos (5)
    it->lnum_pixel_width (28)

x_produce_glyphs:
    it->c (9)
    hscroll (2)
    it->first_visible_x (14)
    it->current_x (7)
    it->hpos (0)
    it->lnum_pixel_width (28)

x_produce_glyphs:
    it->c (9)
    hscroll (2)
    it->first_visible_x (14)
    it->current_x (91)
    it->hpos (6)
    it->lnum_pixel_width (28)

Row     Start       End Used oE><\CTZFesm     X    Y    W    H    V    A    P
==============================================================================
 13       486       499   19 111000110000     0  208  153   16   16   12   12
           -1        -1	    0
           -1        -1
           -1        -1
 Glyph#  Type       Pos   O   W     Code      C Face LR
      0     C        -1   0   7 0x000020          27 00
      1     C        -1   0   7 0x000031      1   27 00
      2     C        -1   0   7 0x000034      4   27 00
      3     C        -1   0   7 0x000020          27 00
      4     S       486   B  35 0x000000          29 00
      5     C       487   B   7 0x0000bb      .   30 00
      6     S       487   B  -1 0x000000          29 00
      7     C       488   B   7 0x000048      H    0 00
      8     C       489   B   7 0x000065      e    0 00
      9     C       490   B   7 0x00006c      l    0 00
     10     C       491   B   7 0x00006c      l    0 00
     11     C       492   B   7 0x00006f      o    0 00
     12     C       493   B   7 0x00002d      -    0 00
     13     C       494   B   7 0x000077      w    0 00
     14     C       495   B   7 0x00006f      o    0 00
     15     C       496   B   7 0x000072      r    0 00
     16     C       497   B   7 0x00006c      l    0 00
     17     C       498   B   7 0x000064      d    0 00
     18     C         0   0   7 0x000020           0 00

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

DATE:  [02-19-2018 08:28:38] <19 Feb 2018 18:28:38 +0200>
FROM:  Eli Zaretskii <eliz@gnu.org>
> 
> > Date:  Sun, 18 Feb 2018 18:17:26 -0800
> > From:  Keith David Bershatsky <esq@lawlist.com>
> > Cc:  30226@debbugs.gnu.org
> > 
> > A major break-through in the tracing of bug 30226 ....  it->current_x differs between the real thing (when display_line calls PRODUCE_GLYPHS), versus the simulation (when move_it_in_display_line_to calls PRODUCE_GLYPHS), _because_ maybe_produce_line_number advances it->current_x in the former but _not_ the latter.
> 
> How do you see that maybe_produce_line_number doesn't advance
> current_x?  AFAICS, it does that unconditionally:
> 
>   for ( ; g < e; g++)
>     {
>       it->current_x += g->pixel_width;  <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
>       /* The following is important when this function is called
>   from move_it_in_display_line_to: HPOS is incremented only
>   when we are in the visible portion of the glyph row.  */
>       if (it->current_x > it->first_visible_x)
>  it->hpos++;
>       if (p)
>  {
>    *p++ = *g;
>    (*u)++;
>  }
>     }
> 
> Did you mean hpos instead?  Or did you mean that
> maybe_produce_line_number is not called from
> move_it_in_display_line_to in some case, whereas it is called from
> display_line?  Or something else?
> 
> > SOLUTION (suggested):  Revise move_it_in_display_line_to advance X by simulating a call to maybe_produce_line_number and permitting that function to advance X.
> 
> But this is already done...


[-- Attachment #2: patch.diff --]
[-- Type: application/diff, Size: 6705 bytes --]