emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Fill column indicator functionality


From: Eli Zaretskii
Subject: Re: Fill column indicator functionality
Date: Mon, 11 Mar 2019 17:30:47 +0200

> Date: Mon, 11 Mar 2019 11:48:14 +0100
> From: Ergus <address@hidden>
> Cc: address@hidden
> 
> 1) In the graphical implementation I generated the column following the
> Eli suggestions with something like: 
> 
>     struct font *font = face->font ? face->font : FRAME_FONT (f);
> 
>     it->char_to_display = '|';
>     int stretch_ascent = (((it->ascent + it->descent)
>               * FONT_BASE (font)) / FONT_HEIGHT (font));
> 
>     memset (&it->position, 0, sizeof it->position);
>     it->avoid_cursor_p = true;
>     it->face_id = merge_faces (it->w, Qline_number, 0, DEFAULT_FACE_ID);
>     it->start_of_box_run_p = false;
>     append_stretch_glyph (it, Qnil, width,
>                           it->ascent + it->descent, stretch_ascent);
> 
>     PRODUCE_GLYPHS (it);
> 
>     it->position = saved_pos;
>     it->avoid_cursor_p = saved_avoid_cursor;
>     it->face_id = saved_face_id;
>     it->start_of_box_run_p = saved_box_start;
>     it->char_to_display = saved_char;
> 
>  The problem is that in this case if (setq show-trailing-whitespace t)
> the whitespaces aren't highlighted if they are not longer that the line;
> in term mode (with the loop) it works fine.
> 
> The function highlight_trailing_whitespace checks for glyph->type ==
> STRETCH_GLYPH so I am missing something. 

I think you are missing this:

      /* If last glyph is a space or stretch, and it's trailing
         whitespace, set the face of all trailing whitespace glyphs in
         IT->glyph_row to `trailing-whitespace'.  */
      if ((row->reversed_p ? glyph <= start : glyph >= start)
          && BUFFERP (glyph->object)  <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
          && (glyph->type == STRETCH_GLYPH
              || (glyph->type == CHAR_GLYPH
                  && glyph->u.ch == ' '))
          && trailing_whitespace_p (glyph->charpos))

This test is for those stretch glyphs that came from a buffer,
i.e. those generated by TABs and 'space' display properties.  But in
your case, the object of the stretch glyph is nil, as specified by the
2nd argument of append_stretch_glyph:

    append_stretch_glyph (it, Qnil, width,
                          it->ascent + it->descent, stretch_ascent);

Moreover, I believe you are looking at the wrong code fragment to
explain why trailing-whitespace stopped working.  I think the problem
happens a bit earlier, here:

      /* Skip over glyphs inserted to display the cursor at the
         end of a line, for extending the face of the last glyph
         to the end of the line on terminals, and for truncation
         and continuation glyphs.  */
      if (!row->reversed_p)
        {
          while (glyph >= start
                 && glyph->type == CHAR_GLYPH
                 && NILP (glyph->object))
            --glyph;
        }

This skips only glyphs of type CHAR_GLYPH, but your change inserts a
STRETCH_GLYPH, which you also want to skip.  So the above condition
should be augmented to include STRETCH_GLYPH as well.

> 2) What name do you suggest for this mode? I propose
> display-column-indicator.

display-fill-column-indicator-mode?

Thanks again for working on this.

P.S. Thanks to you, I just uncovered an old bug, whereby
show-trailing-whitespace didn't work in R2L screen lines, and for the
same reason: the code didn't account for the stretch glyphs inserted
by the display engine at the left edge of such lines.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]