new :extend face attribute

[andreyorst]

With latest commits in Emacs 27, there's new :extend face attribute, that is for extending highlighting beyond EOL. By default it isn't set to t and now doom-one looks like this:

Should I manually patch every face, or there will be a fix for that? It seems DOOM Emacs can use Emacs 27 as a base, so I wonder where changes should be put.

[hlissner]

Will Emacs ignore :extend in earlier versions? Or throw an error about unknown properties?

[andreyorst]

it throws error in Emacs 26.3

[andreyorst]

I've opened a bug at Emacs: #37774

[andreyorst]

This is the set of faces I've manually set up to make it look as it was before :extend introduced.

(use-package doom-themes
  :commands (doom-themes-org-config)
  :config
  (doom-themes-org-config)
  (setq doom-themes-enable-bold t
        doom-themes-enable-italic t)
  (when (>= emacs-major-version 27)
    (with-eval-after-load 'org
      (dolist (face '(org-block
                      org-block-begin-line
                      org-block-end-line
                      org-level-1))
        (set-face-attribute face nil :extend t)))
    (with-eval-after-load 'magit
      (dolist (face '(magit-diff-hunk-heading
                      magit-diff-hunk-heading-highlight
                      magit-diff-hunk-heading-selection
                      magit-diff-hunk-region
                      magit-diff-lines-heading
                      magit-diff-lines-boundary
                      magit-diff-conflict-heading
                      magit-diff-added
                      magit-diff-removed
                      magit-diff-our
                      magit-diff-base
                      magit-diff-their
                      magit-diff-context
                      magit-diff-added-highlight
                      magit-diff-removed-highlight
                      magit-diff-our-highlight
                      magit-diff-base-highlight
                      magit-diff-their-highlight
                      magit-diff-context-highlight
                      magit-diff-whitespace-warning
                      magit-diffstat-added
                      magit-diffstat-removed
                      magit-section-heading
                      magit-section-heading-selection
                      magit-section-highlight
                      magit-section-secondary-heading
                      magit-diff-file-heading
                      magit-diff-file-heading-highlight
                      magit-diff-file-heading-selection))
        (set-face-attribute face nil :extend t)))
    (with-eval-after-load 'ediff
      (dolist (face '(ediff-current-diff-A
                      ediff-current-diff-Ancestor
                      ediff-current-diff-B
                      ediff-current-diff-C
                      ediff-even-diff-A
                      ediff-even-diff-Ancestor
                      ediff-even-diff-B
                      ediff-even-diff-C
                      ediff-fine-diff-A
                      ediff-fine-diff-Ancestor
                      ediff-fine-diff-B
                      ediff-fine-diff-C
                      ediff-odd-diff-A
                      ediff-odd-diff-Ancestor
                      ediff-odd-diff-B
                      ediff-odd-diff-C))
        (set-face-attribute face nil :extend t)))
    (with-eval-after-load 'hl-line
      (set-face-attribute 'hl-line nil :extend t))
    (with-eval-after-load 'faces
      (dolist (face '(region
                      secondary-selection))
        (set-face-attribute face nil :extend t))))
  :init (load-theme 'doom-one t))

I obviously missed some, but maybe this will help narrow down the amount a bit

[seagle0128]

FYI. I saw this post https://www.reddit.com/r/emacs/comments/diahh1/emacs_27_update_changed_how_highlighted_lines/.

@bandresen @hlissner I tested this snippet in Emacs25 and no error occurs.

(use-package hl-line
  :ensure nil
  :custom-face (hl-line ((t (:extend t))))
  :hook (after-init . global-hl-line-mode))

or

(custom-set-faces '(hl-line ((t (:extend t)))))

[cireu]

@andreyorst Thx you so much!

I find helm and macrostep also need this

(with-eval-after-load 'helm
  (set-face-extend 'helm-selection t))
(with-eval-after-load 'macrostep
  (set-face-extend 'macrostep-expansion-highlight-face t))

[hlissner]

I tested this snippet in Emacs25 and no error occurs.

@seagle0128 Oh, that changes things.

I'll soon look into adding :extend properties to the faces mentioned here. Let me know if there are any others!

[andreyorst]

I find that many transient faces are now not extended, like the line at the bottom. But I don't know if this should be reported to transient directly, since, I believe that it was extended without doom-themes, and doom should just inherit the faces?

[seagle0128]

I don't understand why the default value isn't nil. That could keep compatibilities.

[andreyorst]

As discussed in #37774 the decision was that amount of faces that need to be extended is much less, so they assumed that this change would not affect much:

Eli Zaretskii It was intentional, meaning the assumption was that extending the face past the last character on the line makes little sense in general. IOW, preventing the extension for most faces was the main point of the change.

Actually, it seems that this statement is correct. Very little faces in core packages need this. And we can't calculate the amount of faces for external packages.

On a side note, this chage seem fixed this bug, so workaround not needed anymore:

https://github.com/hlissner/doom-emacs/blob/d297dc6934301a2b9597b03426b9f56bee09282e/modules/ui/doom/config.el#L62-L76

At least for me.

[cireu]

I don't understand why the default value isn't nil. That could keep compatibilities.

Eli consider this is the way get the things done correctly.

See discussion here https://lists.gnu.org/archive/html/bug-gnu-emacs/2019-10/msg01809.html

[cireu]

It seems that they surely want to do this change for 27. We can take the action to add :extend t for faces now.

[seagle0128]

I see a trick from magit.

https://github.com/magit/magit/commit/891ebdca58ef498ff3b94c8ac2921cc6b72c3d25

`((t ,@(and (>= emacs-major-version 27) '(:extend t))

[hlissner]

As of 8cd0516 this issue should be resolved. Let me know if that isn't the case or if there are faces/themes I've missed. Thanks for bringing it to my attention!

[wedens]

I'm not sure where it should be fixed, so I won't open a new issue yet, but using highlight-indent-guides with (setq highlight-indent-guides-method 'character) makes cursor disappear at the end of the line with indent guides.

[wedens]

Unfortunately this problem doesn't seem to be fixed either.

[hlissner]

@wedens Please try the fixes mentioned in hlissner/doom-emacs/issues/1988 on the faces of highlight-indent-guides. I don't use Emacs 27 nor highlight-indent-guides, so it'd be a big help.

Also, 6e3526b should fix hlissner/doom-emacs/issues/1988.

[wedens]

Also, 6e3526b should fix hlissner/doom-emacs/issues/1988.

I could reproduce the problem without solaire. But actually I've just updated emacs to the latest commit (5a3e96b17c2a948ac952295962dc6e281ec5cad5) and the problem is fixed.

UPD: the problem with highlight-indent-guides seems to also be fixed :tada:

[andreyorst]

As of 8cd0516 this issue should be resolved. Let me know if that isn't the case or if there are faces/themes I've missed. Thanks for bringing it to my attention!

I think markdown-mode faces should be tweaked as well: Official screenshot shows this:

[hlissner]

@andreyorst Fixed

[cireu]

http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=58fb4c3e68a4a42ad491d0fa2c084e5c39942e2b

Accroding to this commit, it's unnecessary for us to repeat :extend t in themes, it will inherit from original spec! :D

[hlissner]

Fantastic. I'll give it a month or three for folks to catch up to this version, then I'll remove them from the package.