unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Problems with move_it_in_display_line_to X when tabs exist.
@ 2017-11-29  6:12 Keith David Bershatsky
  2017-11-29 18:04 ` Eli Zaretskii
  0 siblings, 1 reply; 22+ messages in thread
From: Keith David Bershatsky @ 2017-11-29  6:12 UTC (permalink / raw)
  To: Emacs Devel

I have reached a road block calculating X and HPOS when dealing with tabs while working on feature requests 17684 (crosshairs) and 22873 (multiple fake cursors).

DETAILS:  Native line numbers are visible.  No overlays are present.  No special text properties are present.

GOAL:  The goal is to move IT from it.first_visible_x to it.last_visible_x on the line containing POINT and place fake cursors on each tab, adjustable tab-width, and/or character.  As I move IT over the line containing POINT, I want to reliably obtain X and HPOS.  This works well if there are no tabs.  And, it works well if there are just a couple of tabs with a tab-width of 2.  At some point, however, it stops working when there are more than just a couple of tabs on the same line, especially when they are mixed somewhere within the middle of the line.  I am using draw_window_cursor to create multiple fake cursors (with the underscore shape) on the entire current line, to create a horizontal ruler spanning the window-body-width.

I can see that X and/or HPOS are wrong when tabs are present on the current line because I get superimposed letters and double of the same word slightly off to the left and/or the right of where the word should be.

HYPOTHESIS:  I would venture to say that what is displayed on screen does _not_ coincide with what IT reports when running move_it_in_display_line_to.

IDEAS:  Erase the entire current line and redraw it so that what is displayed on screen matches exactly what IT reports when running move_it_in_display_line_to.

Any additional ideas or thoughts on how to deal with tabs in this scenario would be greatly appreciated.


SNIPPET USED TO CREATE HORIZONTAL LINE:

  while (true)
    {
      flavor = (it.c != '\t') ? Qmc_glyph : Qmc_glyph_tab;
      if (hscl)
        {
          relative_x = it.current_x - (hscl_first_hpos * frame_char_width);
          hpos = it.hpos - hscl_first_hpos;
        }
        else
          {
            relative_x = it.current_x - first_x;
            hpos = it.hpos;
          }
      if (header_line_height > 0)
        vpos = it.vpos + 1;
        else
          vpos = it.vpos;
      bool lpw_reached_p = ((hscl
                             && it.current_x >= hscl_first_x + lnum_pixel_width)
                            || (!hscl
                                && it.current_x >= first_x + lnum_pixel_width));
      bool final_loop_p = (ITERATOR_AT_END_OF_LINE_P (&it)
                           || FETCH_BYTE (IT_BYTEPOS (it)) == '\n'
                           || rc == MOVE_POS_MATCH_OR_ZV);
      bool tab_invisible_p = (it.c == '\t');
      bool tab_visible_p = (it.c != '\t'
                            && FETCH_BYTE (IT_BYTEPOS (it)) == '\t');
      bool real_fake_cursor_p = (opoint_x == relative_x
                                 && opoint_y == it.current_y
                                 && opoint_hpos == hpos
                                 && opoint_vpos == vpos);
      if (!real_fake_cursor_p
          && lpw_reached_p
          && !tab_invisible_p)
        draw_fake_cursor (w, relative_x, it.current_y, hpos, vpos,
                          HBAR_CURSOR, cursor_width,
                          foreground, color_vector (w, it.face_id),
                          flavor, IT_CHARPOS (it), &result);
      if (!real_fake_cursor_p
          && lpw_reached_p
          && tab_invisible_p)
        {
          int i, count = it.pixel_width / frame_char_width;
          for (i = 0; i < count; i++)
            {
              draw_fake_cursor (w, relative_x, it.current_y, hpos, vpos,
                                HBAR_CURSOR, cursor_width,
                                foreground, color_vector (w, it.face_id),
                                flavor, IT_CHARPOS (it), &result);
              relative_x = relative_x + frame_char_width;
            }
        }
      if (final_loop_p)
        break;
      rc = mc_move_it_in_display_line (w, &it, it.current_x + it.pixel_width);
      if (rc == MOVE_LINE_CONTINUED)
        break;
      relative_x = (hscl)
                   ? it.current_x - (hscl_first_hpos * frame_char_width)
                   : it.current_x - first_x;
      if (relative_x + frame_char_width >= text_area_width)
        break;
    }


int
mc_move_it_in_display_line (struct window *w, struct it *it, int target_x)
{
  if (IT_CHARPOS (*it) == ZV)
    return MOVE_POS_MATCH_OR_ZV;
  struct it saved_it;
  void *saved_data = bidi_shelve_cache ();
  enum move_it_result rc = MOVE_X_REACHED;
  int new_x, prev_x;
  /* When `auto-hscroll-mode' is set to `current-line` and we are horizontal scrolling
  a long line that approaches or exceeds an `it.current.x` of approximately 1000, `rc`
  will erroneously return early with a MOVE_LINE_TRUNCATED indicator without pushing
  forwards until IT reaches the target_x.  As a workaround, ignore MOVE_LINE_TRUNCATED. */
  while (it->current_x + it->pixel_width <= target_x
         && (rc == MOVE_X_REACHED
             || rc == MOVE_LINE_TRUNCATED
             || (it->line_wrap == WORD_WRAP
                 && rc == MOVE_POS_MATCH_OR_ZV)))
    {
      SAVE_IT (saved_it, *it, saved_data);
      new_x = it->current_x + it->pixel_width;
      if (new_x == it->current_x)
        new_x++;
      rc = move_it_in_display_line_to (it, ZV, new_x, MOVE_TO_POS | MOVE_TO_X);
      if (ITERATOR_AT_END_OF_LINE_P (it)
          || FETCH_BYTE (IT_BYTEPOS (*it)) == '\n'
          /* #28936:  `move_it_in_display_line_to' returns MOVE_POS_MATCH_OR_ZV
          before reaching ZV when the latter is at the end of the line AND `word-wrap'
          is non-nil:  abcdefg[ZV].  The workaround is to add an extra check using
          IT_CHARPOS and comparing it to ZV. */
          || (rc == MOVE_POS_MATCH_OR_ZV
              && IT_CHARPOS (*it) == ZV))
        break;
    }
  /* When word-wrap is on, TO_X may lie past the end of a wrapped line.
  Then it->current is the character on the next line, so backtrack to the
  space before the wrap point.  */
  if (it->line_wrap == WORD_WRAP
      && rc == MOVE_LINE_CONTINUED)
    {
      prev_x = max (it->current_x - 1, 0);
      RESTORE_IT (it, &saved_it, saved_data);
      move_it_in_display_line_to (it, -1, prev_x, MOVE_TO_X);
    }
  bidi_unshelve_cache (saved_data, true);
  /* Workaround for bug #28936 -- correct the erroneous MOVE_POS_MATCH_OR_ZV. */
  if (it->current_x == target_x
      && rc == MOVE_POS_MATCH_OR_ZV
      && IT_CHARPOS (*it) != ZV)
    rc = MOVE_X_REACHED;
  return rc;
}



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Problems with move_it_in_display_line_to X when tabs exist.
  2017-11-29  6:12 Keith David Bershatsky
@ 2017-11-29 18:04 ` Eli Zaretskii
  0 siblings, 0 replies; 22+ messages in thread
From: Eli Zaretskii @ 2017-11-29 18:04 UTC (permalink / raw)
  To: Keith David Bershatsky; +Cc: emacs-devel

> Date: Tue, 28 Nov 2017 22:12:35 -0800
> From: Keith David Bershatsky <esq@lawlist.com>
> 
> I can see that X and/or HPOS are wrong when tabs are present on the current line because I get superimposed letters and double of the same word slightly off to the left and/or the right of where the word should be.
> 
> HYPOTHESIS:  I would venture to say that what is displayed on screen does _not_ coincide with what IT reports when running move_it_in_display_line_to.

Does this happen only when line numbers are displayed?

In any case, your description provides a lot of details that are hard
to reason about, because most of the code is not shown.  OTOH, if
indeed there's a bug in move_it_in_display_line_to, then presenting
evidence for it is very simple: show a line of text and its line
number, and then show the X and HPOS values for each tab in that line
as calculated by move_it_in_display_line_to vs the same values in a
displayed line.  (For the latter, you can use the pgrowx command
defined in src/.gdbinit, or manually display the values using the
debugger.)

If you do that, any mismatches between displaying a line and moving
through it with move_it_in_display_line_to will be immediately
apparent, and it should be easy to fix them.



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Problems with move_it_in_display_line_to X when tabs exist.
@ 2017-11-30  4:29 Keith David Bershatsky
  0 siblings, 0 replies; 22+ messages in thread
From: Keith David Bershatsky @ 2017-11-30  4:29 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Thank you, Eli, for your help.

I will work on this over the next few days as time permits and report back.

Keith

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

DATE:  [11-29-2017 10:04:14] <29 Nov 2017 20:04:14 +0200>
FROM:  Eli Zaretskii <eliz@gnu.org>
> 
> > Date: Tue, 28 Nov 2017 22:12:35 -0800
> > From: Keith David Bershatsky <esq@lawlist.com>
> > 
> > I can see that X and/or HPOS are wrong when tabs are present on the current line because I get superimposed letters and double of the same word slightly off to the left and/or the right of where the word should be.
> > 
> > HYPOTHESIS:  I would venture to say that what is displayed on screen does _not_ coincide with what IT reports when running move_it_in_display_line_to.
> 
> Does this happen only when line numbers are displayed?
> 
> In any case, your description provides a lot of details that are hard
> to reason about, because most of the code is not shown.  OTOH, if
> indeed there's a bug in move_it_in_display_line_to, then presenting
> evidence for it is very simple: show a line of text and its line
> number, and then show the X and HPOS values for each tab in that line
> as calculated by move_it_in_display_line_to vs the same values in a
> displayed line.  (For the latter, you can use the pgrowx command
> defined in src/.gdbinit, or manually display the values using the
> debugger.)
> 
> If you do that, any mismatches between displaying a line and moving
> through it with move_it_in_display_line_to will be immediately
> apparent, and it should be easy to fix them.



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Problems with move_it_in_display_line_to X when tabs exist.
@ 2017-12-02 19:52 Keith David Bershatsky
  2017-12-02 21:14 ` Eli Zaretskii
  0 siblings, 1 reply; 22+ messages in thread
From: Keith David Bershatsky @ 2017-12-02 19:52 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

The test assumes:

* The variable word-wrap is non-nil.

* The variable tab-width is 2.

* The buffer-display table entry for a tab is:  [(187 . 120) (9 . 121)].

* Native line numbers are _not_ enabled.

I have a long line that wraps at the window edge and I put a space to simulate a new word that should be wrapped to the following line.  To make it easier to read, I am using the backslash symbol (below) to signify a word-wrap with a space at the window edge.  The next line has three "x" followed by a tab and then the word "hello-world".  The STRETCH glyph is 22 pixels wide; however, it.pixel_width is erroneously (?) reporting that it is 11 pixels wide.

I have hard-coded an emacs_abort () when it.c == '\t', which means that we have just moved passed character 187 (aka "»") and we are now on a STRETCH tab-width.

Here is a link to a screen-shot of this particular issue:

https://www.lawlist.com/images/pgrowx_a.png

What would be the next step(s), please, to continue working on debugging this particular issue?

xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx \
xxx	hello-world


(gdb) pgrowx glyph_row
[New Thread 0x1c0b of process 16745]
TEXT: 18 glyphs
  0    0: CHAR[x] pos=1197 blev=0,btyp=UNDEF w=11 a+d=16+4 MB
  1   11: CHAR[x] pos=1198 blev=0,btyp=UNDEF w=11 a+d=16+4 MB
  2   22: CHAR[x] pos=1199 blev=0,btyp=UNDEF w=11 a+d=16+4 MB
  3   33: CHAR[0xbb] pos=1200 blev=0,btyp=UNDEF w=11 a+d=16+4 face=31 MB
  4   44: STRETCH[20+16] pos=1200 w=22 a+d=16+4 face=30 MB
  5   66: CHAR[h] pos=1201 blev=0,btyp=UNDEF w=11 a+d=16+4 MB
  6   77: CHAR[e] pos=1202 blev=0,btyp=UNDEF w=11 a+d=16+4 MB
  7   88: CHAR[l] pos=1203 blev=0,btyp=UNDEF w=11 a+d=16+4 MB
  8   99: CHAR[l] pos=1204 blev=0,btyp=UNDEF w=11 a+d=16+4 MB
  9  110: CHAR[o] pos=1205 blev=0,btyp=UNDEF w=11 a+d=16+4 MB
 10  121: CHAR[-] pos=1206 blev=0,btyp=UNDEF w=11 a+d=16+4 MB
 11  132: CHAR[w] pos=1207 blev=0,btyp=UNDEF w=11 a+d=16+4 MB
 12  143: CHAR[o] pos=1208 blev=0,btyp=UNDEF w=11 a+d=16+4 MB
 13  154: CHAR[r] pos=1209 blev=0,btyp=UNDEF w=11 a+d=16+4 MB
 14  165: CHAR[l] pos=1210 blev=0,btyp=UNDEF w=11 a+d=16+4 MB
 15  176: CHAR[d] pos=1211 blev=0,btyp=UNDEF w=11 a+d=16+4 MB
 16  187: CHAR[0xb6] pos=1212 blev=0,btyp=UNDEF w=11 a+d=16+4 face=26 MB
 17  198: CHAR[ ] pos=0 blev=0,btyp=UNDEF w=11 a+d=16+4 MB

(gdb) print it.c
$1 = 9

(gdb) print it.pixel_width
$2 = 11

Thanks,

Keith

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

DATE:  [11-29-2017 10:04:14] <29 Nov 2017 20:04:14 +0200>
FROM:  Eli Zaretskii <eliz@gnu.org>
> 
> > Date: Tue, 28 Nov 2017 22:12:35 -0800
> > From: Keith David Bershatsky <esq@lawlist.com>
> > 
> > I can see that X and/or HPOS are wrong when tabs are present on the current line because I get superimposed letters and double of the same word slightly off to the left and/or the right of where the word should be.
> > 
> > HYPOTHESIS:  I would venture to say that what is displayed on screen does _not_ coincide with what IT reports when running move_it_in_display_line_to.
> 
> Does this happen only when line numbers are displayed?
> 
> In any case, your description provides a lot of details that are hard
> to reason about, because most of the code is not shown.  OTOH, if
> indeed there's a bug in move_it_in_display_line_to, then presenting
> evidence for it is very simple: show a line of text and its line
> number, and then show the X and HPOS values for each tab in that line
> as calculated by move_it_in_display_line_to vs the same values in a
> displayed line.  (For the latter, you can use the pgrowx command
> defined in src/.gdbinit, or manually display the values using the
> debugger.)
> 
> If you do that, any mismatches between displaying a line and moving
> through it with move_it_in_display_line_to will be immediately
> apparent, and it should be easy to fix them.



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Problems with move_it_in_display_line_to X when tabs exist.
  2017-12-02 19:52 Keith David Bershatsky
@ 2017-12-02 21:14 ` Eli Zaretskii
  0 siblings, 0 replies; 22+ messages in thread
From: Eli Zaretskii @ 2017-12-02 21:14 UTC (permalink / raw)
  To: Keith David Bershatsky; +Cc: emacs-devel

> Date:  Sat, 02 Dec 2017 11:52:33 -0800
> From:  Keith David Bershatsky <esq@lawlist.com>
> Cc:  emacs-devel@gnu.org
> 
> The test assumes:
> 
> * The variable word-wrap is non-nil.
> 
> * The variable tab-width is 2.
> 
> * The buffer-display table entry for a tab is:  [(187 . 120) (9 . 121)].
> 
> * Native line numbers are _not_ enabled.
> 
> I have a long line that wraps at the window edge and I put a space to simulate a new word that should be wrapped to the following line.  To make it easier to read, I am using the backslash symbol (below) to signify a word-wrap with a space at the window edge.  The next line has three "x" followed by a tab and then the word "hello-world".  The STRETCH glyph is 22 pixels wide; however, it.pixel_width is erroneously (?) reporting that it is 11 pixels wide.
> 
> I have hard-coded an emacs_abort () when it.c == '\t', which means that we have just moved passed character 187 (aka "»") and we are now on a STRETCH tab-width.

What is the value of it.what and of it.char_to_display?  And what is
the value of it.method and it.current_x?

My crystal ball says that, since your display table shows a TAB as 2
characters, the first of which is a u+00bb, and is displayed as an
11-pixel glyph, that is the display element whose metrics you are
seeing.  The stretch glyph produced by the TAB from the display table
is after that.



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Problems with move_it_in_display_line_to X when tabs exist.
@ 2017-12-02 22:28 Keith David Bershatsky
  2017-12-03  3:32 ` Eli Zaretskii
  0 siblings, 1 reply; 22+ messages in thread
From: Keith David Bershatsky @ 2017-12-02 22:28 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Here are the four values.  It looks like it.current_x is where I expected it to be, but it.what is showing a character instead of a stretch.

(gdb) print it.what
$1 = IT_CHARACTER

(gdb) print it.char_to_display
$2 = 9

(gdb) print it.method
$3 = GET_FROM_DISPLAY_VECTOR

(gdb) print it.current_x
$4 = 44

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

DATE:  [12-02-2017 13:14:33] <02 Dec 2017 23:14:33 +0200>
FROM:  Eli Zaretskii <eliz@gnu.org>
> 
> > Date:  Sat, 02 Dec 2017 11:52:33 -0800
> > From:  Keith David Bershatsky <esq@lawlist.com>
> > Cc:  emacs-devel@gnu.org
> > 
> > The test assumes:
> > 
> > * The variable word-wrap is non-nil.
> > 
> > * The variable tab-width is 2.
> > 
> > * The buffer-display table entry for a tab is:  [(187 . 120) (9 . 121)].
> > 
> > * Native line numbers are _not_ enabled.
> > 
> > I have a long line that wraps at the window edge and I put a space to simulate a new word that should be wrapped to the following line.  To make it easier to read, I am using the backslash symbol (below) to signify a word-wrap with a space at the window edge.  The next line has three "x" followed by a tab and then the word "hello-world".  The STRETCH glyph is 22 pixels wide; however, it.pixel_width is erroneously (?) reporting that it is 11 pixels wide.
> > 
> > I have hard-coded an emacs_abort () when it.c == '\t', which means that we have just moved passed character 187 (aka "»") and we are now on a STRETCH tab-width.
> 
> What is the value of it.what and of it.char_to_display?  And what is
> the value of it.method and it.current_x?
> 
> My crystal ball says that, since your display table shows a TAB as 2
> characters, the first of which is a u+00bb, and is displayed as an
> 11-pixel glyph, that is the display element whose metrics you are
> seeing.  The stretch glyph produced by the TAB from the display table
> is after that.



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Problems with move_it_in_display_line_to X when tabs exist.
  2017-12-02 22:28 Keith David Bershatsky
@ 2017-12-03  3:32 ` Eli Zaretskii
  0 siblings, 0 replies; 22+ messages in thread
From: Eli Zaretskii @ 2017-12-03  3:32 UTC (permalink / raw)
  To: Keith David Bershatsky; +Cc: emacs-devel

> Date:  Sat, 02 Dec 2017 14:28:31 -0800
> From:  Keith David Bershatsky <esq@lawlist.com>
> Cc:  emacs-devel@gnu.org
> 
> Here are the four values.  It looks like it.current_x is where I expected it to be, but it.what is showing a character instead of a stretch.
> 
> (gdb) print it.what
> $1 = IT_CHARACTER
> 
> (gdb) print it.char_to_display
> $2 = 9
> 
> (gdb) print it.method
> $3 = GET_FROM_DISPLAY_VECTOR
> 
> (gdb) print it.current_x
> $4 = 44

Please show the Lisp code that you used to set up the display table.



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Problems with move_it_in_display_line_to X when tabs exist.
@ 2017-12-03  3:38 Keith David Bershatsky
  2017-12-03 14:29 ` Eli Zaretskii
  0 siblings, 1 reply; 22+ messages in thread
From: Keith David Bershatsky @ 2017-12-03  3:38 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

The five faces were set up with the built-in defface macro.  I set the buffer-display-table with setq and the following variable:

(defvar +-buffer-display-table
  (let* (
      (display-table (make-display-table))
      (ff-char
        (cond
          ((eq system-type 'darwin)
            ?\U0001D4D5)
          ((eq system-type 'windows-nt)
            ?\u0046)))
      (glyph-form-feed (make-glyph-code ff-char '+-form-feed-face))
      (glyph-pilcrow (make-glyph-code ?\u00B6 '+-newline-face))
      (glyph-space (make-glyph-code ?\u00B7 '+-space-face))
      (glyph-tab (make-glyph-code ?\u00BB '+-tab-face))
      (glyph-tab-spacer (make-glyph-code ?\t '+-tab-spacer-face)))
    (aset display-table ?\n `[,glyph-pilcrow ?\n])
    (aset display-table ?\f `[,glyph-form-feed])
    (aset display-table ?\t `[,glyph-tab ,glyph-tab-spacer])
    (aset display-table ?\s `[,glyph-space])
    display-table)
  "The `buffer-display-table' that is used when `+-mode' is active.")

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

DATE:  [12-02-2017 19:32:49] <03 Dec 2017 05:32:49 +0200>
FROM:  Eli Zaretskii <eliz@gnu.org>
> 
> > Date:  Sat, 02 Dec 2017 14:28:31 -0800
> > From:  Keith David Bershatsky <esq@lawlist.com>
> > Cc:  emacs-devel@gnu.org
> > 
> > Here are the four values.  It looks like it.current_x is where I expected it to be, but it.what is showing a character instead of a stretch.
> > 
> > (gdb) print it.what
> > $1 = IT_CHARACTER
> > 
> > (gdb) print it.char_to_display
> > $2 = 9
> > 
> > (gdb) print it.method
> > $3 = GET_FROM_DISPLAY_VECTOR
> > 
> > (gdb) print it.current_x
> > $4 = 44
> 
> Please show the Lisp code that you used to set up the display table.



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Problems with move_it_in_display_line_to X when tabs exist.
  2017-12-03  3:38 Keith David Bershatsky
@ 2017-12-03 14:29 ` Eli Zaretskii
  0 siblings, 0 replies; 22+ messages in thread
From: Eli Zaretskii @ 2017-12-03 14:29 UTC (permalink / raw)
  To: Keith David Bershatsky; +Cc: emacs-devel

> Date:  Sat, 02 Dec 2017 19:38:51 -0800
> From:  Keith David Bershatsky <esq@lawlist.com>
> Cc:  emacs-devel@gnu.org
> 
> The five faces were set up with the built-in defface macro.  I set the buffer-display-table with setq and the following variable:
> 
> (defvar +-buffer-display-table
>   (let* (
>       (display-table (make-display-table))
>       (ff-char
>         (cond
>           ((eq system-type 'darwin)
>             ?\U0001D4D5)
>           ((eq system-type 'windows-nt)
>             ?\u0046)))
>       (glyph-form-feed (make-glyph-code ff-char '+-form-feed-face))
>       (glyph-pilcrow (make-glyph-code ?\u00B6 '+-newline-face))
>       (glyph-space (make-glyph-code ?\u00B7 '+-space-face))
>       (glyph-tab (make-glyph-code ?\u00BB '+-tab-face))
>       (glyph-tab-spacer (make-glyph-code ?\t '+-tab-spacer-face)))
>     (aset display-table ?\n `[,glyph-pilcrow ?\n])
>     (aset display-table ?\f `[,glyph-form-feed])
>     (aset display-table ?\t `[,glyph-tab ,glyph-tab-spacer])
>     (aset display-table ?\s `[,glyph-space])
>     display-table)
>   "The `buffer-display-table' that is used when `+-mode' is active.")

I cannot reproduce your problem with the latest emacs-26 branch.

The above snippet leaves a lot undefined, so I simulated that as
follows:

  (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 'fringe)))
  (setq word-wrap t)
  (set tab-width 2)

Then I inserted a long line as you described, arranging for its second
screen line to start with "xxx" followed by a TAB.

Then I emulated your calls to move_it_in_display_line_to by setting
visual-order-cursor-movement to t, putting the cursor on the 0xBB
character which depicts the TAB, and typing "M-x right-char RET".
If I set a breakpoint in xdisp.c:22746, which is here:

      else if (it.current_x != target_x)
	move_it_in_display_line_to (&it, ZV, target_x, MOVE_TO_POS | MOVE_TO_X);

I see that this call returns with it.current on the stretch glyph
following the 0xBB character, and with it.pixel_width set correctly to
the width of the stretch glyph.

So I think either you call move_it_in_display_line_to in some way that
is different from the above, or there's something else you didn't tell
in your recipe.



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Problems with move_it_in_display_line_to X when tabs exist.
@ 2017-12-03 20:56 Keith David Bershatsky
  2017-12-04 15:48 ` Eli Zaretskii
  0 siblings, 1 reply; 22+ messages in thread
From: Keith David Bershatsky @ 2017-12-03 20:56 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

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

Thank you, Eli, for trying to reproduce the current issue.  The good news is that I was able to reproduce the issue with Emacs 26, dated today (12/03/2017), bearing last commit 1fdac2d65c7134a2e1261bf7ad32d275f37da0dc.  The bad news is that my recipe is cumbersome.  In the coming days, I will work on developing a less cumbersome recipe.

For the issue to be present, the tab STRETCH must _visibly_ be 2 columns wide _and_ the tab CHARACTER must be 1 column wide, for a grand total of 3 columns.  The variable tab-width is exactly 2.  The preceding long line must have a space at the window edge; and, the following line contains a second word with just "xxx", a tab, followed by some letters like "hello-world".

This evening (after work), I will experiment using your recipe in the most recent email (below).  I can also build Emacs 26 on Windows and compare the results to Emacs 26 on OSX 10.6.8 (which is what I currently use).

Here is my complete recipe for Emacs 26:

0.  Screen-shot taken today:

https://lawlist.com/images/pgrowx_b.png

1.  Apply the attached patch.diff, and build a modified Emacs 26.  I am using the following settings:

CFLAGS='-O0 -g3' ./configure \
--with-ns \
--enable-checking='yes,glyphs' \
--enable-check-lisp-object-type \
--without-compress-install \
--without-makeinfo \
--with-gnutls=no \
--with-mailutils \
--without-makeinfo

2.  Launch the newly built modified Emacs 26 in gdb.

3.  Open the attached Lisp library named +-mode.el and evaluate the entire buffer.

4.  Open the attached .scratch file; place the cursor at the end of the buffer; and type M-x +-mode

[If the default font is different than Emacs 26 on OSX 10.6.8, then the long line will need to be adjusted such that the last character at the right window edge is a space.]

5.  In the gdb debugger select the frame that threw the hard-code emacs_abort and run the tests that we have been using:

Breakpoint 1, terminate_due_to_signal (sig=6, backtrace_limit=40)
    at emacs.c:364
364	  signal (sig, SIG_DFL);

(gdb) frame 2
#2  0x000000010002d244 in mc_draw_crosshairs (w=0x103be7038) at xdisp.c:1918
1918	emacs_abort ();

(gdb) pgrowx glyph_row
[New Thread 0x1d0b of process 25723]
TEXT: 17 glyphs
  0    0: CHAR[x] pos=1099 blev=0,btyp=L w=7 a+d=12+4 MB
  1    7: CHAR[x] pos=1100 blev=0,btyp=L w=7 a+d=12+4 MB
  2   14: CHAR[x] pos=1101 blev=0,btyp=L w=7 a+d=12+4 MB
  3   21: CHAR[0xbb] pos=1102 blev=0,btyp=L w=7 a+d=12+4 face=36 MB
  4   28: STRETCH[16+12] pos=1102 w=14 a+d=12+4 face=35 MB
  5   42: CHAR[h] pos=1103 blev=0,btyp=L w=7 a+d=12+4 MB
  6   49: CHAR[e] pos=1104 blev=0,btyp=L w=7 a+d=12+4 MB
  7   56: CHAR[l] pos=1105 blev=0,btyp=L w=7 a+d=12+4 MB
  8   63: CHAR[l] pos=1106 blev=0,btyp=L w=7 a+d=12+4 MB
  9   70: CHAR[o] pos=1107 blev=0,btyp=L w=7 a+d=12+4 MB
 10   77: CHAR[-] pos=1108 blev=0,btyp=L w=7 a+d=12+4 MB
 11   84: CHAR[w] pos=1109 blev=0,btyp=L w=7 a+d=12+4 MB
 12   91: CHAR[o] pos=1110 blev=0,btyp=L w=7 a+d=12+4 MB
 13   98: CHAR[r] pos=1111 blev=0,btyp=L w=7 a+d=12+4 MB
 14  105: CHAR[l] pos=1112 blev=0,btyp=L w=7 a+d=12+4 MB
 15  112: CHAR[d] pos=1113 blev=0,btyp=L w=7 a+d=12+4 MB
 16  119: CHAR[ ] pos=0 blev=0,btyp=B w=7 a+d=12+4 MB

(gdb) print it.c
$1 = 9

(gdb) print it.pixel_width
$2 = 7

(gdb) print it.what
$3 = IT_CHARACTER

(gdb) print it.method
$4 = GET_FROM_DISPLAY_VECTOR

(gdb) print it.current_x
$5 = 28

(gdb) print it.char_to_display
$6 = 9

(gdb) print it.method


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

DATE:  [12-03-2017 06:29:57] <03 Dec 2017 16:29:57 +0200>
FROM:  Eli Zaretskii <eliz@gnu.org>
> 
> > Date:  Sat, 02 Dec 2017 19:38:51 -0800
> > From:  Keith David Bershatsky <esq@lawlist.com>
> > Cc:  emacs-devel@gnu.org
> > 
> > The five faces were set up with the built-in defface macro.  I set the buffer-display-table with setq and the following variable:
> > 
> > (defvar +-buffer-display-table
> >   (let* (
> >       (display-table (make-display-table))
> >       (ff-char
> >         (cond
> >           ((eq system-type 'darwin)
> >             ?\U0001D4D5)
> >           ((eq system-type 'windows-nt)
> >             ?\u0046)))
> >       (glyph-form-feed (make-glyph-code ff-char '+-form-feed-face))
> >       (glyph-pilcrow (make-glyph-code ?\u00B6 '+-newline-face))
> >       (glyph-space (make-glyph-code ?\u00B7 '+-space-face))
> >       (glyph-tab (make-glyph-code ?\u00BB '+-tab-face))
> >       (glyph-tab-spacer (make-glyph-code ?\t '+-tab-spacer-face)))
> >     (aset display-table ?\n `[,glyph-pilcrow ?\n])
> >     (aset display-table ?\f `[,glyph-form-feed])
> >     (aset display-table ?\t `[,glyph-tab ,glyph-tab-spacer])
> >     (aset display-table ?\s `[,glyph-space])
> >     display-table)
> >   "The `buffer-display-table' that is used when `+-mode' is active.")
> 
> I cannot reproduce your problem with the latest emacs-26 branch.
> 
> The above snippet leaves a lot undefined, so I simulated that as
> follows:
> 
>   (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 'fringe)))
>   (setq word-wrap t)
>   (set tab-width 2)
> 
> Then I inserted a long line as you described, arranging for its second
> screen line to start with "xxx" followed by a TAB.
> 
> Then I emulated your calls to move_it_in_display_line_to by setting
> visual-order-cursor-movement to t, putting the cursor on the 0xBB
> character which depicts the TAB, and typing "M-x right-char RET".
> If I set a breakpoint in xdisp.c:22746, which is here:
> 
>       else if (it.current_x != target_x)
>  move_it_in_display_line_to (&it, ZV, target_x, MOVE_TO_POS | MOVE_TO_X);
> 
> I see that this call returns with it.current on the stretch glyph
> following the 0xBB character, and with it.pixel_width set correctly to
> the width of the stretch glyph.
> 
> So I think either you call move_it_in_display_line_to in some way that
> is different from the above, or there's something else you didn't tell
> in your recipe.


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

[-- Attachment #3: .scratch --]
[-- Type: application/javascript, Size: 1113 bytes --]

[-- Attachment #4: +-mode.el --]
[-- Type: application/el, Size: 30548 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Problems with move_it_in_display_line_to X when tabs exist.
@ 2017-12-04  3:01 Keith David Bershatsky
  2017-12-04 16:26 ` Eli Zaretskii
  0 siblings, 1 reply; 22+ messages in thread
From: Keith David Bershatsky @ 2017-12-04  3:01 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

I built Emacs 26 for Windows (using my modification patch) and ran the same series of tests along with taking a screen-shot at the link below.  it.pixel_width is correct on Emacs 26 for Windows, whereas it.pixel_width is wrong on Emacs 26 for OSX 10.6.8.  it.what reports IT_CHARACTER on both platforms, however, I expected the result to be IT_STRETCH.  Later this evening, I will experiment with the recipe that you tried in your previous email.

SCREEN-SHOT:

https://www.lawlist.com/images/pgrowx_c.png


GDB:

(gdb) frame 2
#2  0x01027098 in mc_draw_crosshairs (w=0x53df718) at xdisp.c:1917
1917    emacs_abort ();

(gdb) pgrowx glyph_row
TEXT: 17 glyphs
  0    0: CHAR[x] pos=1102 blev=0,btyp=L w=8 a+d=12+4 MB
  1    8: CHAR[x] pos=1103 blev=0,btyp=L w=8 a+d=12+4 MB
  2   16: CHAR[x] pos=1104 blev=0,btyp=L w=8 a+d=12+4 MB
  3   24: CHAR[0xbb] pos=1105 blev=0,btyp=L w=8 a+d=12+4 face=30 MB
  4   32: STRETCH[16+12] pos=1105 w=16 a+d=12+4 face=29 MB
  5   48: CHAR[h] pos=1106 blev=0,btyp=L w=8 a+d=12+4 MB
  6   56: CHAR[e] pos=1107 blev=0,btyp=L w=8 a+d=12+4 MB
  7   64: CHAR[l] pos=1108 blev=0,btyp=L w=8 a+d=12+4 MB
  8   72: CHAR[l] pos=1109 blev=0,btyp=L w=8 a+d=12+4 MB
  9   80: CHAR[o] pos=1110 blev=0,btyp=L w=8 a+d=12+4 MB
 10   88: CHAR[-] pos=1111 blev=0,btyp=L w=8 a+d=12+4 MB
 11   96: CHAR[w] pos=1112 blev=0,btyp=L w=8 a+d=12+4 MB
 12  104: CHAR[o] pos=1113 blev=0,btyp=L w=8 a+d=12+4 MB
 13  112: CHAR[r] pos=1114 blev=0,btyp=L w=8 a+d=12+4 MB
 14  120: CHAR[l] pos=1115 blev=0,btyp=L w=8 a+d=12+4 MB
 15  128: CHAR[d] pos=1116 blev=0,btyp=L w=8 a+d=12+4 MB
 16  136: CHAR[ ] pos=0 blev=0,btyp=B w=8 a+d=12+4 MB

(gdb) print it.pixel_width
$1 = 16

(gdb) print it.what
$2 = IT_CHARACTER

(gdb) print it.current_x
$3 = 32

(gdb) print it.method
$4 = GET_FROM_DISPLAY_VECTOR

(gdb) print it.c
$5 = 9

(gdb) print it.char_to_display
$6 = 9



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Problems with move_it_in_display_line_to X when tabs exist.
@ 2017-12-04  8:03 Keith David Bershatsky
  0 siblings, 0 replies; 22+ messages in thread
From: Keith David Bershatsky @ 2017-12-04  8:03 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

I could use a little help, please, to perform the same test that you did.  I built Emacs 26 tonight _without_ any modifications for both OSX and Windows.  I launched Emacs 26 on both platforms with no user configuration using gdb.  From gdb, I set a breakpoint as follows:

break xdisp.c:22746

I created a long word-wrapped line with a single space at the right window edge, followed by a continued line with three (3) xxx, followed by a tab, followed by hello-world.  I evaluated the following code (using setq instead of set for the tab-width):

(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 'fringe)))
(setq word-wrap t)
(setq tab-width 2)
(setq visual-order-cursor-movement t)

I placed by my cursor on the 00BB character (aka »), and I typed M-x right-char.

On both platforms, IT is on the second "x" from the far left of the window edge.  In other words, I appear to be moving in the wrong direction and when I go to print the it.*** info in gdb, I am clearly on the second "x" -- i.e., I am one character to the _left_ of ».

Thanks,

Keith

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

DATE:  [12-03-2017 06:29:57] <03 Dec 2017 16:29:57 +0200>
FROM:  Eli Zaretskii <eliz@gnu.org>
> 
> * * *
> 
> The above snippet leaves a lot undefined, so I simulated that as
> follows:
> 
>   (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 'fringe)))
>   (setq word-wrap t)
>   (set tab-width 2)
> 
> Then I inserted a long line as you described, arranging for its second
> screen line to start with "xxx" followed by a TAB.
> 
> Then I emulated your calls to move_it_in_display_line_to by setting
> visual-order-cursor-movement to t, putting the cursor on the 0xBB
> character which depicts the TAB, and typing "M-x right-char RET".
> If I set a breakpoint in xdisp.c:22746, which is here:
> 
>       else if (it.current_x != target_x)
>  move_it_in_display_line_to (&it, ZV, target_x, MOVE_TO_POS | MOVE_TO_X);
> 
> I see that this call returns with it.current on the stretch glyph
> following the 0xBB character, and with it.pixel_width set correctly to
> the width of the stretch glyph.
> 
> So I think either you call move_it_in_display_line_to in some way that
> is different from the above, or there's something else you didn't tell
> in your recipe.



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Problems with move_it_in_display_line_to X when tabs exist.
  2017-12-03 20:56 Keith David Bershatsky
@ 2017-12-04 15:48 ` Eli Zaretskii
  0 siblings, 0 replies; 22+ messages in thread
From: Eli Zaretskii @ 2017-12-04 15:48 UTC (permalink / raw)
  To: Keith David Bershatsky; +Cc: emacs-devel

> Date:  Sun, 03 Dec 2017 12:56:49 -0800
> From:  Keith David Bershatsky <esq@lawlist.com>
> Cc:  emacs-devel@gnu.org
> 
> For the issue to be present, the tab STRETCH must _visibly_ be 2 columns wide _and_ the tab CHARACTER must be 1 column wide, for a grand total of 3 columns.  The variable tab-width is exactly 2.  The preceding long line must have a space at the window edge; and, the following line contains a second word with just "xxx", a tab, followed by some letters like "hello-world".

That's exactly what I did.



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Problems with move_it_in_display_line_to X when tabs exist.
  2017-12-04  3:01 Keith David Bershatsky
@ 2017-12-04 16:26 ` Eli Zaretskii
  0 siblings, 0 replies; 22+ messages in thread
From: Eli Zaretskii @ 2017-12-04 16:26 UTC (permalink / raw)
  To: Keith David Bershatsky; +Cc: emacs-devel

> Date:  Sun, 03 Dec 2017 19:01:47 -0800
> From:  Keith David Bershatsky <esq@lawlist.com>
> Cc:  emacs-devel@gnu.org
> 
> I built Emacs 26 for Windows (using my modification patch) and ran the same series of tests along with taking a screen-shot at the link below.  it.pixel_width is correct on Emacs 26 for Windows, whereas it.pixel_width is wrong on Emacs 26 for OSX 10.6.8.

I'm surprised you see a difference.  This code is supposed to be
platform independent.  So I guess you need to examine all the places
in xdisp.c which are "#ifded NS" or "#ifndef NS".

> it.what reports IT_CHARACTER on both platforms, however, I expected the result to be IT_STRETCH.

This is expected, and not a bug: IT_STRETCH means we are producing
stretch glyphs "out of thin air", this happens when processing 'space'
display properties and line-prefixes.  When we display a TAB, we
produce the stretch glyph from a TAB, which is a character.



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Problems with move_it_in_display_line_to X when tabs exist.
@ 2017-12-06 16:24 Keith David Bershatsky
  0 siblings, 0 replies; 22+ messages in thread
From: Keith David Bershatsky @ 2017-12-06 16:24 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

I wanted to give you an update on my efforts.  I was unable to isolate different results due to the platform upon which Emacs was built/run.  I did, however, track down a few areas in Emacs 26 where it->pixel_width is being set incorrectly when I am running my custom code.  When the continuation_lines_width is incorrect, the code in the vicinity of line 28212 sets an erroneous it.pixel_width.  Inasmuch as I am moving along the length of the current line by it.pixel_width, the X and/or HPOS coordinates get skewed.  I tried manually setting continuation_lines_width to 0 at both locations mentioned below, and it.pixel_width gets reported correctly.  However, this solution is not ideal because the STRETCH does not shrink and expand as it would normally do as things change in the buffer; e.g., 
 adding/removing the right vertical scroll-bar affects whether the STRETCH would normally be either 1 or 2 columns wide.  I feel as though I am zeroing-in on the underlying issue, but more work is ne
 eded in the coming days/weeks as time permits:

xdisp.c:9563
it->continuation_lines_width += it->current_x;

xdisp.c:21671
it->continuation_lines_width += x;

xdisp.c:28212
it->pixel_width = next_tab_x - x0;

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

DATE:  [12-04-2017 08:26:04] <04 Dec 2017 18:26:04 +0200>
FROM:  Eli Zaretskii <eliz@gnu.org>
> 
> > Date:  Sun, 03 Dec 2017 19:01:47 -0800
> > From:  Keith David Bershatsky <esq@lawlist.com>
> > Cc:  emacs-devel@gnu.org
> > 
> > I built Emacs 26 for Windows (using my modification patch) and ran the same series of tests along with taking a screen-shot at the link below.  it.pixel_width is correct on Emacs 26 for Windows, whereas it.pixel_width is wrong on Emacs 26 for OSX 10.6.8.
> 
> I'm surprised you see a difference.  This code is supposed to be
> platform independent.  So I guess you need to examine all the places
> in xdisp.c which are "#ifded NS" or "#ifndef NS".
> 
> * * *



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Problems with move_it_in_display_line_to X when tabs exist.
@ 2018-01-15  5:48 Keith David Bershatsky
  2018-01-15 12:06 ` Eli Zaretskii
  0 siblings, 1 reply; 22+ messages in thread
From: Keith David Bershatsky @ 2018-01-15  5:48 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

I am still working on troubleshooting how the value of it->pixel_width is determined.  In the current fact pattern, I am using the following settings:

  (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)

I am placing a tab at flush-left and some text following the tab, such as:

	Hello-world.

I am calling interactively (scroll-left 1) to temporarily scroll the text to the left 1 column at a time.

I believe that x_produce_glyphs sets the it->pixel-width of the stretch tab incorrectly (while scrolling 2 or more columns to the left), which affects the ability to properly move by it.pixel_width when calling move_it_in_display_line_to.  As far as I can tell, this problem _only_ happens when displaying native line numbers.

The problem likely begins at line 28228 of xdisp.c where we have a comment:

/* i.e. (it->char_to_display == '\t') */

I am trying to understand that section of code.

QUESTION:  Is this line of code:

  int next_tab_x = ((1 + x + tab_width - 1) / tab_width) * tab_width;

the same thing as:

  int next_tab_x = x + tab_width;

If we add 1 and then subtract 1, we did not do anything meaningful.

If we divide something by tab_width and then multiply it by tab_width, then we are back again at where we started.

EXAMPLE #1:  (1 + 11 + 77 - 1) / 77) * 77 = 88

EXAMPLE #2:  11 + 77 = 88

Thanks,

Keith



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Problems with move_it_in_display_line_to X when tabs exist.
  2018-01-15  5:48 Keith David Bershatsky
@ 2018-01-15 12:06 ` Eli Zaretskii
  0 siblings, 0 replies; 22+ messages in thread
From: Eli Zaretskii @ 2018-01-15 12:06 UTC (permalink / raw)
  To: Keith David Bershatsky; +Cc: emacs-devel

> Date:  Sun, 14 Jan 2018 21:48:12 -0800
> From:  Keith David Bershatsky <esq@lawlist.com>
> Cc:  emacs-devel@gnu.org
> 
> I am still working on troubleshooting how the value of it->pixel_width is determined.  In the current fact pattern, I am using the following settings:
> 
>   (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)
> 
> I am placing a tab at flush-left and some text following the tab, such as:
> 
> 	Hello-world.
> 
> I am calling interactively (scroll-left 1) to temporarily scroll the text to the left 1 column at a time.
> 
> I believe that x_produce_glyphs sets the it->pixel-width of the stretch tab incorrectly (while scrolling 2 or more columns to the left), which affects the ability to properly move by it.pixel_width when calling move_it_in_display_line_to.  As far as I can tell, this problem _only_ happens when displaying native line numbers.

As previously, please provide a complete recipe, with text that needs
to be inserted into a buffer, and the commands that show why you think
the pixel_width is calculated incorrectly.  The above Lisp snippet is
a good start, but the main part, i.e. the text to insert, the commands
to perform, and the values you see produced -- that part is missing.

> QUESTION:  Is this line of code:
> 
>   int next_tab_x = ((1 + x + tab_width - 1) / tab_width) * tab_width;
> 
> the same thing as:
> 
>   int next_tab_x = x + tab_width;

No, it isn't, see below.

> If we add 1 and then subtract 1, we did not do anything meaningful.

True.  The code does that to make more explicitly clear how the value
of 'x + tab_width' was arrived to.  Omitting the two 1's could leave
the reader wondering where did those one-pixel adjustment disappear.

> If we divide something by tab_width and then multiply it by tab_width, then we are back again at where we started.
> 
> EXAMPLE #1:  (1 + 11 + 77 - 1) / 77) * 77 = 88

No, this is integer division, so 88/77 yields 1, and the overall
result is 77, not 88.



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Problems with move_it_in_display_line_to X when tabs exist.
@ 2018-01-16  4:41 Keith David Bershatsky
  2018-01-16 17:00 ` Eli Zaretskii
  0 siblings, 1 reply; 22+ messages in thread
From: Keith David Bershatsky @ 2018-01-16  4:41 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

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

Thank you, Eli, for teaching me about "integer division".  I have added that concept to my learning "todo list" and anticipate having a handle on that in the next couple of days.

GOAL:  The goal is to interactively call (scroll-left 1) -- one or more times in a row -- and obtain the beginning _visible_ X and HPOS of the STRETCH tab; and, also obtain its _visible_ pixel-width.

The following screenshots with stderr results were obtained by calling the function debug-tab-pixel-width, which is contained in the attached patch.diff.  I saw that x_produce_glyphs is able to achieve the correct it->pixel_width for the STRETCH tab when x0 >= it->lnum_pixel_width; however, it is run subsequent in time to when we call move_it_in_display_line_to.


0.  Opening screenshot -- just setting up the test buffer.

  https://www.lawlist.com/images/tab_width_bug_00.png


1.  Place the cursor on the line that begins with a tab, and press the f5 key once.

  https://www.lawlist.com/images/tab_width_bug_01.png

  OBSERVATIONS (w->hscroll == 1):  The expected result is that the STRETCH tab will have an it.pixel_width of 42.  The third (3rd) iteration/loop has the wrong value; i.e., 49.  The fourth (4th) iteration/loop has the correct value; i.e., 42.  x_produce_glyphs runs subsequent in time and contains the desired value of 42 when x0 >= it->lnum_pixel_width.


2.  With the cursor still on the line that began with a tab, Press the f5 key once.

  https://www.lawlist.com/images/tab_width_bug_02.png

  OBSERVATIONS (w->hscroll == 2):  The expected result is that the STRETCH tab will have an it.pixel_width of 35; however, it has a value of 49 instead.  x_produce_glyphs runs subsequent in time and contains the desired value of 35 when x0 >= it->lnum_pixel_width.


3. With the cursor still on the line that began with a tab, Press the f5 key once.

  https://www.lawlist.com/images/tab_width_bug_03.png

  OBSERVATIONS (w->hscroll == 3):  The expected result is that the STRETCH tab will have an it.pixel_width of 28; however, it has a value of 49 instead.  x_produce_glyphs runs subsequent in time and contains the desired value of 28 when x0 >= it->lnum_pixel_width.


LISP CODE (buffer-local):

(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) (debug-tab-pixel-width)))


TEST TEXT (a tab, followed by some arbitrary letters such Hello-world) -- the text begins on line 13:

	Hello-world.


C CODE:  Apply the attached patch.diff to Emacs 26 as of 01/15/2018 bearing last commit 9f22b7d2317eff65897355dcf68ba10d521cfa5a.



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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Problems with move_it_in_display_line_to X when tabs exist.
  2018-01-16  4:41 Keith David Bershatsky
@ 2018-01-16 17:00 ` Eli Zaretskii
  0 siblings, 0 replies; 22+ messages in thread
From: Eli Zaretskii @ 2018-01-16 17:00 UTC (permalink / raw)
  To: Keith David Bershatsky; +Cc: emacs-devel

> Date:  Mon, 15 Jan 2018 20:41:52 -0800
> From:  Keith David Bershatsky <esq@lawlist.com>
> Cc:  emacs-devel@gnu.org
> 
> The following screenshots with stderr results were obtained by calling the function debug-tab-pixel-width, which is contained in the attached patch.diff.  I saw that x_produce_glyphs is able to achieve the correct it->pixel_width for the STRETCH tab when x0 >= it->lnum_pixel_width; however, it is run subsequent in time to when we call move_it_in_display_line_to.
> 
> 
> 0.  Opening screenshot -- just setting up the test buffer.
> 
>   https://www.lawlist.com/images/tab_width_bug_00.png
> 
> 
> 1.  Place the cursor on the line that begins with a tab, and press the f5 key once.
> 
>   https://www.lawlist.com/images/tab_width_bug_01.png
> 
>   OBSERVATIONS (w->hscroll == 1):  The expected result is that the STRETCH tab will have an it.pixel_width of 42.  The third (3rd) iteration/loop has the wrong value; i.e., 49.  The fourth (4th) iteration/loop has the correct value; i.e., 42.  x_produce_glyphs runs subsequent in time and contains the desired value of 42 when x0 >= it->lnum_pixel_width.

What do you mean by "x_produce_glyphs runs subsequent in time"?  The
value of it->pixel_width is updated by x_produce_glyphs, so before it
was called, that value is outdated (belongs to the previous glyph).
If that is what you observe, then it's the expected behavior, and not
a bug.



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Problems with move_it_in_display_line_to X when tabs exist.
@ 2018-01-16 17:53 Keith David Bershatsky
  0 siblings, 0 replies; 22+ messages in thread
From: Keith David Bershatsky @ 2018-01-16 17:53 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

x_produce_glyphs runs multiple times on the line containing the STRETCH tab.  The value for it->pixel_width of the STRETCH tab (in this example) is _only_ correct when it->current_x is >= it->lnum_pixel_width.  Inasmuch as move_it_in_display_line_to is reporting that it.pixel_width remains at 49 even though the STRETCH shrinks (as we call scroll-left 1) to 42 to 35 to 28, it appears that x_produce_glyphs may be overwriting the correct value with the wrong value.  I restricted the floodgate of STDERR messages coming from x_produce_glyphs to only the situation when x0 >= it->lnum_pixel_width.  x_produce_glyphs probably runs _again_ when x0 < it->lnum_pixel_width, which overwrites the good value with the bad value.  Then, when I run move_it_in_display_line_to in screenshots 02 and 03, the err
 oneous value of 49 is returned -- the correct value in screenshot 02 should be 35, and the correct value in screenshot 03 should be 28.  If the previous value of it->pixel_width for the prior comman
 d loop were correct, then we would not see a consistent value of 49 in schreenshots 02 and 03 -- instead, we would see a value of 42 in screenshot 02 [which is really 35] and we would see a value 35 in screenshot 03 [which is really 28].

It may be the case that it is presently impossible to know the prospective _new_ value of it->pixel_width for a STRETCH tab (in this example) when calling move_it_in_display_line_to.  Since x_produce_glyphs knows how to calculate the correct value of it->pixel_width (when x0 >= it->lnum_pixel_width), perhaps there is a way to teach move_it_in_display_line_to how to do this?

Keith

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

DATE:  [01-16-2018 09:00:43] <16 Jan 2018 19:00:43 +0200>
FROM:  Eli Zaretskii <eliz@gnu.org>
> 
> > Date:  Mon, 15 Jan 2018 20:41:52 -0800
> > From:  Keith David Bershatsky <esq@lawlist.com>
> > Cc:  emacs-devel@gnu.org
> > 
> > The following screenshots with stderr results were obtained by calling the function debug-tab-pixel-width, which is contained in the attached patch.diff.  I saw that x_produce_glyphs is able to achieve the correct it->pixel_width for the STRETCH tab when x0 >= it->lnum_pixel_width; however, it is run subsequent in time to when we call move_it_in_display_line_to.
> > 
> > 
> > 0.  Opening screenshot -- just setting up the test buffer.
> > 
> >   https://www.lawlist.com/images/tab_width_bug_00.png
> > 
> > 
> > 1.  Place the cursor on the line that begins with a tab, and press the f5 key once.
> > 
> >   https://www.lawlist.com/images/tab_width_bug_01.png
> > 
> >   OBSERVATIONS (w->hscroll == 1):  The expected result is that the STRETCH tab will have an it.pixel_width of 42.  The third (3rd) iteration/loop has the wrong value; i.e., 49.  The fourth (4th) iteration/loop has the correct value; i.e., 42.  x_produce_glyphs runs subsequent in time and contains the desired value of 42 when x0 >= it->lnum_pixel_width.
> 
> What do you mean by "x_produce_glyphs runs subsequent in time"?  The
> value of it->pixel_width is updated by x_produce_glyphs, so before it
> was called, that value is outdated (belongs to the previous glyph).
> If that is what you observe, then it's the expected behavior, and not
> a bug.



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Problems with move_it_in_display_line_to X when tabs exist.
@ 2018-01-21 20:32 Keith David Bershatsky
  0 siblings, 0 replies; 22+ messages in thread
From: Keith David Bershatsky @ 2018-01-21 20:32 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

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

I was able to obtain the correct width of the tab STRETCH (in this example) by adding the following snippet to x_produce_glyphs.  However, it.current_x is wrong for the glyphs that _follow_ the tab STRETCH -- the difference is an extra font->space_width.  It is impossible for IT to land on the first character following the tab STRETCH using move_it_in_display_line_to -- i.e., IT stops one (1) character to the right of where it should be.

I have revised the function debug-tab-pixel-width to include the output of dump_glyph_row so that we can see the values for it->pixel_width are wrong as to the tab STRETCH width.  [We force a redisplay _before_ calling dump_glyph_row so that the values reflect what we actually see on the screen.]  There are seven (7) new screen-shots that include the STDERR output to the terminal as we increase the value of w->hscroll by a factor of 1 in each subsequent image; i.e., as we call (scroll-left 1) again and again.  Attached is a revised patch.diff to run similar tests as we did before, but now using the new method of calculating the tab STRETCH width.

Any ideas regarding how to fix the erroneous it.current_x as to glyphs that _follow_ the tab STRETCH would be greatly appreciated.  [Other IT values may be incorrect as well, but I've only noticed problems with the pixel_width and current_x.]

Also, there may be a better way to properly calculate the correct value of the tab STRETCH in this example?  I used a new pointer of it->my_pixel_width so as not to break anything in the process while working on this issue.

dispextern.h (new snippet added):  int my_pixel_width;

xdisp.c (new snippet added):

  if (it->char_to_display == '\t'
      && !NILP (Vdisplay_line_numbers)
      && it->w->hscroll > 0
      && it->current_x < it->lnum_pixel_width)
    {
      int my_tab_width = it->tab_width * font->space_width;
      int my_x = it->current_x + it->continuation_lines_width;
      int my_next_tab_x = ((1 + my_x + my_tab_width - 1) / my_tab_width)
                          * my_tab_width;
      if (my_next_tab_x - my_x < font->space_width)
        my_next_tab_x += my_tab_width;
      if (!NILP (Vdisplay_line_numbers))
        my_next_tab_x += it->lnum_pixel_width
                         - ((it->w->hscroll * font->space_width)
                            % my_tab_width);
      /* FIXME:  Should we use font->space_width instead of it->current_x? */
      it->my_pixel_width = my_next_tab_x - it->lnum_pixel_width - it->current_x;
    }
    else
      it->my_pixel_width = 0;

GOAL:  The goal is to interactively call (scroll-left 1) -- one or more times in a row -- and obtain the beginning _visible_ X and HPOS of the tab STRETCH; and, also obtain its _visible_ pixel-width.


00.  Opening screen-shot -- just setting up the test buffer.

  https://www.lawlist.com/images/debug_tab_width_00.png


01.  Place the cursor at the end of the line containing "	@" and press the f5 key one time.

  https://www.lawlist.com/images/debug_tab_width_01.png

  OBSERVATIONS (w->hscroll == 1):  There is a hiccup on loops 3 and 4 (see screen-shot) -- perhaps this is due to the wrong value of it.pixel_width when calling:

  move_it_in_display_line_to (&it, ZV, it.current_x + it.pixel_width,
                              MOVE_TO_POS | MOVE_TO_X);

The expected result is that the tab STRETCH will have an it.pixel_width of 42.  The third (3rd) iteration/loop has the wrong value; i.e., 49.  The fourth (4th) iteration/loop has the correct value (i.e., 42) -- this is the _only_ occasion where we see the correct value of it.pixel_width (in this example).  When w->hscroll > 1, it.pixel_width is _never_ correct as to the tab STRETCH (in this example).


02.  With the cursor at the end of the line containing "	@", press the f5 key one time.

  https://www.lawlist.com/images/debug_tab_width_02.png

  OBSERVATIONS (w->hscroll == 2):  The expected result is that the tab STRETCH will have an it.pixel_width of 35; however, it has a value of 49 instead.


03. With the cursor at the end of the line containing "	@", press the f5 key one time.

  https://www.lawlist.com/images/debug_tab_width_03.png

  OBSERVATIONS (w->hscroll == 3):  The expected result is that the tab STRETCH will have an it.pixel_width of 28; however, it has a value of 49 instead.


04. With the cursor at the end of the line containing "	@", press the f5 key one time.

  https://www.lawlist.com/images/debug_tab_width_04.png

  OBSERVATIONS (w->hscroll == 4):  The expected result is that the tab STRETCH will have an it.pixel_width of 21; however, it has a value of 49 instead.


05. With the cursor at the end of the line containing "	@", press the f5 key one time.

  https://www.lawlist.com/images/debug_tab_width_05.png

  OBSERVATIONS (w->hscroll == 5):  The expected result is that the tab STRETCH will have an it.pixel_width of 14; however, it has a value of 49 instead.


06. With the cursor at the end of the line containing "	@", press the f5 key one time.

  https://www.lawlist.com/images/debug_tab_width_06.png

  OBSERVATIONS (w->hscroll == 6):  The expected result is that the tab STRETCH will have an it.pixel_width of 7; however, it has a value of 49 instead.


LISP CODE (buffer-local):

(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) (debug-tab-pixel-width)))


TEST TEXT (a tab, followed by the "@" symbol) -- the text begins on line 13:

	@


C CODE:  Apply the attached patch.diff to Emacs 26 as of 01/21/2018 bearing last commit c965e5a641d9478d23a233b48977503506b1b603.


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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Problems with move_it_in_display_line_to X when tabs exist.
@ 2018-01-23  7:38 Keith David Bershatsky
  0 siblings, 0 replies; 22+ messages in thread
From: Keith David Bershatsky @ 2018-01-23  7:38 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Inasmuch as I have now resolved the values of it->pixel_width and it->current_x in this example, I felt it was time to move this discussion over to a formal bug report with a tracking number:

http://debbugs.gnu.org/cgi/bugreport.cgi?bug=30226

Thanks,

Keith



^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2018-01-23  7:38 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-06 16:24 Problems with move_it_in_display_line_to X when tabs exist Keith David Bershatsky
  -- strict thread matches above, loose matches on Subject: below --
2018-01-23  7:38 Keith David Bershatsky
2018-01-21 20:32 Keith David Bershatsky
2018-01-16 17:53 Keith David Bershatsky
2018-01-16  4:41 Keith David Bershatsky
2018-01-16 17:00 ` Eli Zaretskii
2018-01-15  5:48 Keith David Bershatsky
2018-01-15 12:06 ` Eli Zaretskii
2017-12-04  8:03 Keith David Bershatsky
2017-12-04  3:01 Keith David Bershatsky
2017-12-04 16:26 ` Eli Zaretskii
2017-12-03 20:56 Keith David Bershatsky
2017-12-04 15:48 ` Eli Zaretskii
2017-12-03  3:38 Keith David Bershatsky
2017-12-03 14:29 ` Eli Zaretskii
2017-12-02 22:28 Keith David Bershatsky
2017-12-03  3:32 ` Eli Zaretskii
2017-12-02 19:52 Keith David Bershatsky
2017-12-02 21:14 ` Eli Zaretskii
2017-11-30  4:29 Keith David Bershatsky
2017-11-29  6:12 Keith David Bershatsky
2017-11-29 18:04 ` Eli Zaretskii

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).