Re: Merging the underline attribute at EOL

[Ergus <spacibba@aol.com>]

Hi Eli:

Actually this seems to be related with the face merge we do latter to
fill the space until the window edge and how we calculate
extend_face_id.

It is taking the text-properties in the call to face_at_pos just before
the condition you mention.

So the problem seems to be in face_at_buffer_position in these lines:

   /* Merge in attributes specified via text properties.  */
   if (!NILP (prop))
     merge_face_ref (w, f, prop, attrs, true, NULL, attr_filter);

So maybe that is the condition we need to extend...

As a dumb test I just did:

if (!NILP (prop) && attr_filter > 0)
     merge_face_ref (w, f, prop, attrs, true, NULL, attr_filter);

And it seems to solve this specific issue (in tui and gui)... but it is
inconsistent and I am not aware of the possible side effects...

Or probably we must fix this inside merge_face_ref. What do you think?



On Sat, Dec 14, 2019 at 10:28:45AM +0200, Eli Zaretskii wrote:
>Jimmy,
>
>The current code in extend_face_to_end_of_line says:
>
>  /* Face extension extends the background and box of IT->extend_face_id
>     to the end of the line.  If the background equals the background
>     of the frame, we don't have to do anything.  */
>  struct face *face = FACE_FROM_ID (f, (it->face_before_selective_p
>                                        ? it->saved_face_id
>                                        : extend_face_id));
>
>  if (FRAME_WINDOW_P (f)
>      && MATRIX_ROW_DISPLAYS_TEXT_P (it->glyph_row)
>      && face->box == FACE_NO_BOX
>      && FACE_COLOR_TO_PIXEL (face->background, f) == FRAME_BACKGROUND_PIXEL (f)
>#ifdef HAVE_WINDOW_SYSTEM
>      && !face->stipple
>#endif
>      && !it->glyph_row->reversed_p
>      && !Vdisplay_fill_column_indicator)
>    return;
>
>This has the effect that the underline property is not extended past
>EOL, and neither are overline and strike-through.  Only the box
>attribute is extended.  Was this how we intended things to be, or is
>this just an oversight?
>
>Currently, this creates some strange counter-intuitive effects.  For
>example, try this in "emacs -Q":
>
>  C-p
>  M-x font-lock-mode RET
>  M-: (add-text-properties (point) (1+ (point)) '(face (:underline t :extend t))) RET
>
>You will see that the underline is limited only to the newline at
>point, it is not extended to the edge of the window.  But if you now
>do this:
>
>  C-SPC
>  C-n
>
>suddenly the entire last line is underlined, in addition to having the
>background color from the region face.
>
>If you replace the :underline with :box in the above example, then the
>last line has the box attribute extended to EOL even before setting
>the region, as expected.
>
>So I think this is a bug, and we should add conditions to the above
>'if' clause.  Am I missing something?
>