bug#37774: 27.0.50; new :extend attribute broke visuals of all themes and other packages

[Ergus]

On Wed, Oct 16, 2019 at 10:46:55PM +0300, Juri Linkov wrote:
> > > 1. Backward-compatibility problem:
> > >
> > > I had to spend significant time investigating why the region face broke
> > > recently, and discovered that customized faces in custom-set-faces need
> > > to be updated.
> >
> > I'm not sure I understand: the region face is defined to be extended
> > beyond EOL.  How does custom-set-faces enter this picture, and why did
> > you need to do anything about the customized faces?
>
> The region face customized long ago in the init file
> has no ':extend t' face attribute, e.g.
>
> (custom-set-faces
> '(region ((((class color) (background light)) (:background "gray90"))))
When moving to a new emacs version (with early init and other tweaks) it
will be recommended to update some details in the init file any way. 

> > > Soon I tired fixing their customizations one by one manually,
> >
> > Which other faces needed to be "fixed", how, and why?
>
> All diff faces and faces that have a distinct background color
> like 'comint-highlight-input' (should extend to window edge
> to help locating visually the command line in shell buffers),
> 'org-block' (because it highlights code blocks), 'xref-file-header'
> for the same reason as diff faces, i.e. faces that highlights blocks.
For sure some other faces will be corrected once we find they are
"broken" or they work better with ":extend t". Org mode is a very active
package so the authors will correct this once they get emacs 27 (or we
contact them). Actually some of them follow this mailing list.

> > Why are you talking only about the colors?  face extension is not only
> > about colors, it's about other attributes as well: underline,
> > strike-through, box, etc.  You list underline with foreground color,
> > but they are not the same as color, especially not when face extension
> > is concerned.  They actually behave more like background colors.
>
> Yes, this new feature is useful for all these face attributes
> to extend them to EOL.  The only exception is background colors.
>
> All complaints are only about extending background colors to EOL.
> So the change could apply to all face attributes except background colors.
I think this will be inconsistent with the own intention if the feature,
the problems it fixes and the criteria to distinguish and determine what
to extend and what not. Remember we are dealing with merged faces too.

> Only other attributes should be extended to EOL, because when such face
> attributes like underline and strike-through are displayed over
> an empty space beyond EOL, this looks ugly.
In the discussion previous to the implementation we agreed to give the
freedom to the user to extend or not. Actually we choose a more complex
design to assert give this freedom to the user. Baybe the user wants to
highlight the region using underline, and adjust that to the EOL or
not... now he can. 

> > And then there are faces with both foreground and background colors.
>
> Actually the distinction is not so simple: even some background colors
> need to extend to EOL, such as when used in combination with the 'box'
> face attributes, because when a button takes two lines, extending
> the button box face to the window edge looks ugly.
This depends of the use case; so we won't have a criteria that will fit
all the cases. 

> > > As I see the change was meant to fix only the problem that relates to
> > > faces with distinct foreground, because indeed underlines extended
> > > to the window edge look very ugly.  So the change should affect
> > > only faces with distinct foreground.
> >
> > That wasn't the intent.  the intent was explicitly to cause the change
> > in background color and underline/strikethough/etc. attributes--those
> > which show in the face extension.  Foreground color doesn't show in
> > face extension.
> >
> > > This screenshot demonstrates how badly broken these blocks are now
> > > in diff-mode that it makes harder to read diffs:
> >
> > I'm sorry, but I don't see why it is broken or hard to read.
>
> Because there is no distinctive rectangular header anymore,
> and no diff hunk blocks.
This will be fixed in 2 minutes once we have a set of faces we should
update. But also this will be a matter of preference. 

> > > Ideally to be more nice-looking, background colors in such faces should be
> > > extended to the column defined e.g. by display-fill-column-indicator-column.
> >
> > That would be ugly if the line's text extends beyond the fill-column,
> > no?  Also, it would look even uglier with variable-pitch fonts.
>
> Extending to the fill-column could be an optional feature.
> It won't work with variable-pitch fonts the same way as
> filling to fill-column doesn't work with variable-pitch fonts.
> But if some line's text will extend beyond the fill-column
> with fixed-pitch fonts, this even could help to find long lines
> (like in whitespace-mode).

This will be not implemented for now; it will add too many corner cases
I'm not sure it worth the effort to solve them just for an exceptional
aesthetic use case. Maybe for emacs 28 ;p