Re: Fill column indicator functionality

[Alp Aker <alp.tekin.aker@gmail.com>]

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

>> In graphical interfaces the space after the line is always filled with
>> the background color of the face of the last produced glyph, even if I
>> reset it to the saved value after the generation. The only solution I
>> found so far was to add an extra glyph after reset the face to the
>> default (saved) value, but hopefully there is a better way?
> I don't think I understand what face is "the face of the last produced
> glyph".  is that the face of the indicator character, is that the
> default face, or is that something else?  Can you show a screenshot?
I noticed an issue with non-default backgrounds that span newlines, such 
as happens with region highlighting. I believe I ran into what Ergus is 
describing while working on a fix (see (3) below).  The version of the 
code I looked at was 9dcaa15e5a, from Ergus's Github repo.

You can see the issue with non-default backgrounds in the attached 
screenshots mode-off.png and mode-on.png; activating 
display-fill-column-indicator-mode truncates the highlighting on each 
line.  To fix:

1. The stretch glyph needs to be drawn in the current face, not the 
fill_column face.

2. The default fill-column face should have an unspecified background 
and it should be merged into the current face during display, not into 
the default face.

3. If the fill-column face specifies a background, we need to reset the 
face to the saved face after producing the indicator glyph. Here I found 
it necessary to insert another display element in order for the face 
change to take effect before the background is extended to the end of 
the line. (I used a 0-width stretch glyph.)  Without that, the 
fill-column face is used (see the attached c.png for a screenshot).  I 
believe this need to add another display element at the end of the line 
is what Ergus was asking about.


diff --git a/lisp/faces.el b/lisp/faces.el
index 153e6a208f..6b9980a77f 100644
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -2504,7 +2504,7 @@ line-number-current-line

  ;; Definition stolen from linum.el.
  (defface fill-column
-  '((t :inherit (shadow default)))
+  '((t :inherit (shadow)))
    "Face for displaying fill column indicator line.
  This face is used when `display-fill-column-indicator-mode' is
  non-nil.

diff --git a/src/xdisp.c b/src/xdisp.c
index 8ac4be8dc7..7c4f9889eb 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -20416,21 +20416,23 @@ extend_face_to_end_of_line (struct it *it)
            int stretch_ascent = (((it->ascent + it->descent)
              * FONT_BASE (font)) / FONT_HEIGHT (font));

-          it->char_to_display = 
XFIXNAT(Vdisplay_fill_column_indicator_character);
            memset (&it->position, 0, sizeof it->position);
            it->avoid_cursor_p = true;
-          it->face_id = merge_faces (it->w, Qfill_column, 0, 
DEFAULT_FACE_ID);
            it->start_of_box_run_p = false;
            it->object = Qnil;

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

+          it->char_to_display = 
XFIXNAT(Vdisplay_fill_column_indicator_character);
+          it->face_id = merge_faces (it->w, Qfill_column, 0, 
saved_face_id);
            PRODUCE_GLYPHS (it);

+          it->face_id = saved_face_id;
+          append_stretch_glyph (it, Qnil, 0, it->ascent + it->descent,
+                             stretch_ascent);
            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;
            it->object = save_object;
@@ -20566,7 +20568,7 @@ extend_face_to_end_of_line (struct it *it)
            if (it->current_x == fill_column_indicator_line)
              {
            const int saved_face = it->face_id;
-          it->face_id = merge_faces (it->w, Qfill_column, 0, 
DEFAULT_FACE_ID);
+          it->face_id = merge_faces (it->w, Qfill_column, 0, saved_face);
            it->c = it->char_to_display = 
XFIXNAT(Vdisplay_fill_column_indicator_character);
            PRODUCE_GLYPHS (it);
            it->face_id = saved_face;


[-- Attachment #2: c.png --]
[-- Type: image/png, Size: 34786 bytes --]

[-- Attachment #3: mode-on.png --]
[-- Type: image/png, Size: 48715 bytes --]

[-- Attachment #4: mode-off.png --]
[-- Type: image/png, Size: 47731 bytes --]