Re: Merging the underline attribute at EOL

[Ergus <spacibba@aol.com>]

Hi Eli:

I just saw your message. Actually the problem should be somewhere else a
bit latter in the same function. Because in terminal your code underlies
the entire line. I will give it a look tomorrow.

Best
Ergus

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?
>