Couple of issues with org block meta lines faces

Couple of issues with org block meta lines faces

[Sébastien Miquel]

Hi,

If you start emacs as follows, making sure emacs picks up the latest org 
from org-plus-contrib, and with file.org being the content of this mail,

#+BEGIN_SRC sh :async
emacs -q \
       --eval "(defface org-block-begin-line '((t (:background \"blue\" 
:height 0.8))) \"\")" \
       --eval "(setq org-fontify-whole-block-delimiter-line t)" \
       file.org
#+END_SRC

You may observe the following issues:

  1) begin-line applies to both begin and end lines. This might be 
intended. If you define an org-block-end-line face, it gets applied instead.
  2) org-fontify-whole-block-delimiter-line is ignored. I'm aware I can 
set the :extend t property to the face. If it does nothing, maybe this 
variable should be removed.
  3) The following block has no face applied.
     - This only happens when the line with # at the top is empty
     - The :height part of the face seems to be responsible
     - It also works fine with default org version
  4) If you go to the end of the fontified end_src line (first src 
block), then press enter a couple of times the buffer position has the 
org-block-begin-line face applied (move cursor to see it).

#
#+BEGIN_SRC elisp
auie
auriaest
#+END_SRC

Regards,

Re: Couple of issues with org block meta lines faces

[Kévin Le Gouguec]

Hi!

Some points I think I can help with:

Sébastien Miquel <sebastien.miquel@posteo.eu> writes:

>   1) begin-line applies to both begin and end lines. This might be 
> intended. If you define an org-block-end-line face, it gets applied instead.

Yup, by default org-block-end-line :inherits from org-block-begin-line.

>   2) org-fontify-whole-block-delimiter-line is ignored. I'm aware I can 
> set the :extend t property to the face. If it does nothing, maybe this 
> variable should be removed.

See emacs bug#42184[1].


[1]: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=42184

Bug#42184: org-fontify-whole-*-line in emacs 27 (was: Couple of issues with org block meta lines faces)

[Kévin Le Gouguec]

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

Could Org maintainers weigh in on bug#42184?

To recap my understanding of the issue:

- org-fontify-whole-heading-line controls whether
  org-set-font-lock-defaults includes the final newline in the headline
  regexp (resp. org-fontify-whole-block-delimiter-line with begin/end
  block regexps).

- This assumes that fontifying the final newline is enough to fontify
  everything beyond this newline.

- This assumption is no longer valid with Emacs 27, where this extension
  is opt-in, using the :extend face attribute.

With Eli's help, I proposed a patch for org-mode against the emacs-27
branch that does something similar to what is done for the org-hide
face: when setting up the major mode, depending on those user options,
the :extend attribute is (re)set for the relevant faces (using a
compatibility function defined in org-compat).

I've reattached the patch for convenience.  Does it look sound?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-org-fontify-whole-line-by-setting-face-extension.patch --]
[-- Type: text/x-diff, Size: 1851 bytes --]

From 07d123c548051eb7f6bbac5c7f5a4e4b8411f976 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?K=C3=A9vin=20Le=20Gouguec?= <kevin.legouguec@gmail.com>
Date: Thu, 9 Jul 2020 16:02:49 +0200
Subject: [PATCH] Fix org-fontify-whole-*-line by setting face extension
 (bug#42184)

* lisp/org/org-compat.el (org--set-faces-extend): New function to set
face extension, for Emacs versions where this attribute exists.
* lisp/org/org.el (org-mode): Call it to set the extend attribute of
relevant faces to the correct value.
---
 lisp/org/org-compat.el | 4 ++++
 lisp/org/org.el        | 6 +++++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/lisp/org/org-compat.el b/lisp/org/org-compat.el
index c1aaf17ca2..fcc325e475 100644
--- a/lisp/org/org-compat.el
+++ b/lisp/org/org-compat.el
@@ -101,6 +101,10 @@ org-table1-hline-regexp
   (defun org-time-convert-to-list (time)
     (seconds-to-time (float-time time))))
 
+(defun org--set-faces-extend (faces extend-p)
+  (when (fboundp 'set-face-extend)
+    (mapc (lambda (f) (set-face-extend f extend-p)) faces)))
+
 \f
 ;;; Emacs < 26.1 compatibility
 
diff --git a/lisp/org/org.el b/lisp/org/org.el
index 568f5b9b87..fb31336ea4 100644
--- a/lisp/org/org.el
+++ b/lisp/org/org.el
@@ -4944,7 +4944,11 @@ org-mode
   ;; Try to set `org-hide' face correctly.
   (let ((foreground (org-find-invisible-foreground)))
     (when foreground
-      (set-face-foreground 'org-hide foreground))))
+      (set-face-foreground 'org-hide foreground)))
+  ;; Set face extension as requested.
+  (org--set-faces-extend '(org-block-begin-line org-block-end-line)
+                         org-fontify-whole-block-delimiter-line)
+  (org--set-faces-extend org-level-faces org-fontify-whole-heading-line))
 
 ;; Update `customize-package-emacs-version-alist'
 (add-to-list 'customize-package-emacs-version-alist
-- 
2.27.0


[-- Attachment #3: Type: text/plain, Size: 233 bytes --]



In addition to this issue, I'd like to ask about the org-block face:
should it be defined with :extend t unconditionally?  As explained in
bug#42184#35, I feel like it should (cf. attached screenshot), but
that's only my opinion.


[-- Attachment #4: org-block.png --]
[-- Type: image/png, Size: 319121 bytes --]

Re: Bug#42184: org-fontify-whole-*-line in emacs 27

[Kévin Le Gouguec]

Since 27.1-rc1 is out, I'd like to bump this; it'd be a shame if 27.1
shipped with this bug, which seems to be getting some attention (I just
spotted a Reddit thread[1] about it, in addition to the original report
on Debbugs).

[1]: https://old.reddit.com/r/emacs/comments/i26n46/why_does_my_orgmode_look_different_to_leuven/


Kévin Le Gouguec <kevin.legouguec@gmail.com> writes:

> Could Org maintainers weigh in on bug#42184?
>
> To recap my understanding of the issue:
>
> - org-fontify-whole-heading-line controls whether
>   org-set-font-lock-defaults includes the final newline in the headline
>   regexp (resp. org-fontify-whole-block-delimiter-line with begin/end
>   block regexps).
>
> - This assumes that fontifying the final newline is enough to fontify
>   everything beyond this newline.
>
> - This assumption is no longer valid with Emacs 27, where this extension
>   is opt-in, using the :extend face attribute.
>
> With Eli's help, I proposed a patch for org-mode against the emacs-27
> branch that does something similar to what is done for the org-hide
> face: when setting up the major mode, depending on those user options,
> the :extend attribute is (re)set for the relevant faces (using a
> compatibility function defined in org-compat).
>
> I've reattached the patch for convenience.  Does it look sound?
>
> From 07d123c548051eb7f6bbac5c7f5a4e4b8411f976 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?K=C3=A9vin=20Le=20Gouguec?= <kevin.legouguec@gmail.com>
> Date: Thu, 9 Jul 2020 16:02:49 +0200
> Subject: [PATCH] Fix org-fontify-whole-*-line by setting face extension
>  (bug#42184)
>
> * lisp/org/org-compat.el (org--set-faces-extend): New function to set
> face extension, for Emacs versions where this attribute exists.
> * lisp/org/org.el (org-mode): Call it to set the extend attribute of
> relevant faces to the correct value.
> ---
>  lisp/org/org-compat.el | 4 ++++
>  lisp/org/org.el        | 6 +++++-
>  2 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/lisp/org/org-compat.el b/lisp/org/org-compat.el
> index c1aaf17ca2..fcc325e475 100644
> --- a/lisp/org/org-compat.el
> +++ b/lisp/org/org-compat.el
> @@ -101,6 +101,10 @@ org-table1-hline-regexp
>    (defun org-time-convert-to-list (time)
>      (seconds-to-time (float-time time))))
>  
> +(defun org--set-faces-extend (faces extend-p)
> +  (when (fboundp 'set-face-extend)
> +    (mapc (lambda (f) (set-face-extend f extend-p)) faces)))
> +
>  \f
>  ;;; Emacs < 26.1 compatibility
>  
> diff --git a/lisp/org/org.el b/lisp/org/org.el
> index 568f5b9b87..fb31336ea4 100644
> --- a/lisp/org/org.el
> +++ b/lisp/org/org.el
> @@ -4944,7 +4944,11 @@ org-mode
>    ;; Try to set `org-hide' face correctly.
>    (let ((foreground (org-find-invisible-foreground)))
>      (when foreground
> -      (set-face-foreground 'org-hide foreground))))
> +      (set-face-foreground 'org-hide foreground)))
> +  ;; Set face extension as requested.
> +  (org--set-faces-extend '(org-block-begin-line org-block-end-line)
> +                         org-fontify-whole-block-delimiter-line)
> +  (org--set-faces-extend org-level-faces org-fontify-whole-heading-line))
>  
>  ;; Update `customize-package-emacs-version-alist'
>  (add-to-list 'customize-package-emacs-version-alist

Re: Bug#42184: org-fontify-whole-*-line in emacs 27

[Kyle Meyer]

Kévin Le Gouguec writes:

> Since 27.1-rc1 is out, I'd like to bump this; it'd be a shame if 27.1
> shipped with this bug, which seems to be getting some attention (I just
> spotted a Reddit thread[1] about it, in addition to the original report
> on Debbugs).

In the associated emacs-bug thread, Eli said that ship has sailed.  With
the possibility of making it into 27.1 out of the picture, I think this
patch should be made against the Org repo, as usual.

As I said in the emacs-bug thread, the patch (which you also included
upstream in this thread) looks fine to me.  If you'd prefer to bundle
your org-block change in the same patch, could you send an updated patch
here against the Org repo?  Or, if you want to send that separately,
there's no need to resend the one you already sent in this thread; I can
adjust it when applying to it to the Org repo.

Re: Bug#42184: org-fontify-whole-*-line in emacs 27

[Kévin Le Gouguec]

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

Kyle Meyer <kyle@kyleam.com> writes:

> Kévin Le Gouguec writes:
>
>> Since 27.1-rc1 is out, I'd like to bump this; it'd be a shame if 27.1
>> shipped with this bug, which seems to be getting some attention (I just
>> spotted a Reddit thread[1] about it, in addition to the original report
>> on Debbugs).
>
> In the associated emacs-bug thread, Eli said that ship has sailed.  With
> the possibility of making it into 27.1 out of the picture, I think this
> patch should be made against the Org repo, as usual.
>
> As I said in the emacs-bug thread, the patch (which you also included
> upstream in this thread) looks fine to me.  If you'd prefer to bundle
> your org-block change in the same patch, could you send an updated patch
> here against the Org repo?

I've just noticed that my patch for org-block (bug#42184#44) was missing
a compatibility shim for Emacs<27; here are updated patches against
maint and master:


[-- Attachment #2: maint.patch --]
[-- Type: text/x-patch, Size: 3171 bytes --]

From 6ca21259589e212452411acd4ac2de3258ede508 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?K=C3=A9vin=20Le=20Gouguec?= <kevin.legouguec@gmail.com>
Date: Fri, 7 Aug 2020 11:04:53 +0200
Subject: [PATCH] Fix org-fontify-whole-*-line by setting face extension
 (bug#42184)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* lisp/org-faces.el (org-block): Set background extension beyond
end-of-line.
* lisp/org-compat.el (org--extended-face): New function to add :extend
attribute to face definition for Emacs≥27.
(org--set-faces-extend): New function to temporarily (re)set :extend
for Emacs≥27.
* lisp/org.el (org-mode): Call it to set the extend attribute of
relevant faces to the correct value.
---
 lisp/org-compat.el | 15 +++++++++++++++
 lisp/org-faces.el  |  2 +-
 lisp/org.el        |  6 +++++-
 3 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/lisp/org-compat.el b/lisp/org-compat.el
index c757355ba..c0f4833a3 100644
--- a/lisp/org-compat.el
+++ b/lisp/org-compat.el
@@ -101,6 +101,21 @@ is nil)."
   (defun org-time-convert-to-list (time)
     (seconds-to-time (float-time time))))
 
+(defmacro org--extended-face (attributes)
+  "Make face that extends beyond end of line.
+
+Up to Emacs 26, all faces extended beyond end of line; getting
+the same behaviour starting with Emacs 27 requires :extend t."
+  `(nconc ,attributes (when (>= emacs-major-version 27) '(:extend t))))
+
+(defun org--set-faces-extend (faces extend-p)
+  "Set the :extend attribute of FACES to EXTEND-P.
+
+This is a no-op for Emacs versions lower than 27, since face
+extension beyond end of line was not controllable."
+  (when (fboundp 'set-face-extend)
+    (mapc (lambda (f) (set-face-extend f extend-p)) faces)))
+
 \f
 ;;; Emacs < 26.1 compatibility
 
diff --git a/lisp/org-faces.el b/lisp/org-faces.el
index 30eab9bc6..4c5a51624 100644
--- a/lisp/org-faces.el
+++ b/lisp/org-faces.el
@@ -393,7 +393,7 @@ follows a #+DATE:, #+AUTHOR: or #+EMAIL: keyword."
   "Face for #+TITLE:, #+AUTHOR:, #+EMAIL: and #+DATE: keywords."
   :group 'org-faces)
 
-(defface org-block '((t :inherit shadow))
+(defface org-block `((t ,(org--extended-face '(:inherit shadow))))
   "Face text in #+begin ... #+end blocks.
 For source-blocks `org-src-block-faces' takes precedence."
   :group 'org-faces
diff --git a/lisp/org.el b/lisp/org.el
index 007dd6e2a..34c0235c1 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -4940,7 +4940,11 @@ The following commands are available:
   ;; Try to set `org-hide' face correctly.
   (let ((foreground (org-find-invisible-foreground)))
     (when foreground
-      (set-face-foreground 'org-hide foreground))))
+      (set-face-foreground 'org-hide foreground)))
+  ;; Set face extension as requested.
+  (org--set-faces-extend '(org-block-begin-line org-block-end-line)
+                         org-fontify-whole-block-delimiter-line)
+  (org--set-faces-extend org-level-faces org-fontify-whole-heading-line))
 
 ;; Update `customize-package-emacs-version-alist'
 (add-to-list 'customize-package-emacs-version-alist
-- 
2.28.0


[-- Attachment #3: master.patch --]
[-- Type: text/x-patch, Size: 3175 bytes --]

From 715927aa2f1baae32040e52d2cfca4aef2edc14b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?K=C3=A9vin=20Le=20Gouguec?= <kevin.legouguec@gmail.com>
Date: Fri, 7 Aug 2020 11:04:53 +0200
Subject: [PATCH] Fix org-fontify-whole-*-line by setting face extension
 (bug#42184)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* lisp/org-faces.el (org-block): Set background extension beyond
end-of-line.
* lisp/org-compat.el (org--extended-face): New function to add :extend
attribute to face definition for Emacs≥27.
(org--set-faces-extend): New function to temporarily (re)set :extend
for Emacs≥27.
* lisp/org.el (org-mode): Call it to set the extend attribute of
relevant faces to the correct value.
---
 lisp/org-compat.el | 15 +++++++++++++++
 lisp/org-faces.el  |  2 +-
 lisp/org.el        |  6 +++++-
 3 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/lisp/org-compat.el b/lisp/org-compat.el
index 5953f89d2..a9cf400a7 100644
--- a/lisp/org-compat.el
+++ b/lisp/org-compat.el
@@ -109,6 +109,21 @@ is nil)."
       (newline-and-indent))
   (defalias 'org-newline-and-indent #'newline-and-indent))
 
+(defmacro org--extended-face (attributes)
+  "Make face that extends beyond end of line.
+
+Up to Emacs 26, all faces extended beyond end of line; getting
+the same behaviour starting with Emacs 27 requires :extend t."
+  `(nconc ,attributes (when (>= emacs-major-version 27) '(:extend t))))
+
+(defun org--set-faces-extend (faces extend-p)
+  "Set the :extend attribute of FACES to EXTEND-P.
+
+This is a no-op for Emacs versions lower than 27, since face
+extension beyond end of line was not controllable."
+  (when (fboundp 'set-face-extend)
+    (mapc (lambda (f) (set-face-extend f extend-p)) faces)))
+
 \f
 ;;; Emacs < 26.1 compatibility
 
diff --git a/lisp/org-faces.el b/lisp/org-faces.el
index d78b606ec..0c7de38ff 100644
--- a/lisp/org-faces.el
+++ b/lisp/org-faces.el
@@ -408,7 +408,7 @@ follows a #+DATE:, #+AUTHOR: or #+EMAIL: keyword."
   "Face for #+TITLE:, #+AUTHOR:, #+EMAIL: and #+DATE: keywords."
   :group 'org-faces)
 
-(defface org-block '((t :inherit shadow))
+(defface org-block `((t ,(org--extended-face '(:inherit shadow))))
   "Face text in #+begin ... #+end blocks.
 For source-blocks `org-src-block-faces' takes precedence."
   :group 'org-faces
diff --git a/lisp/org.el b/lisp/org.el
index ee8be256d..d66977dd4 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -4904,7 +4904,11 @@ The following commands are available:
   ;; Try to set `org-hide' face correctly.
   (let ((foreground (org-find-invisible-foreground)))
     (when foreground
-      (set-face-foreground 'org-hide foreground))))
+      (set-face-foreground 'org-hide foreground)))
+  ;; Set face extension as requested.
+  (org--set-faces-extend '(org-block-begin-line org-block-end-line)
+                         org-fontify-whole-block-delimiter-line)
+  (org--set-faces-extend org-level-faces org-fontify-whole-heading-line))
 
 ;; Update `customize-package-emacs-version-alist'
 (add-to-list 'customize-package-emacs-version-alist
-- 
2.28.0


[-- Attachment #4: Type: text/plain, Size: 528 bytes --]


I don't think anyone has reviewed org--extended-face yet (I first
drafted it in bug#42184#17), so don't hesitate to call me out on it if
something looks off.  FWIW I saw no warnings when running "make" with
Emacs 26.3 nor with 28.

I made patches for both maint and master because

- the patch for maint has a small conflict in org-compat when applied
  against master,

- I'm not sure which branch will eventually be synchronized with
  emacs-27 (assuming we'll include this fix in 27.2).

Thank you for following up on this.

Re: Bug#42184: org-fontify-whole-*-line in emacs 27

[Kyle Meyer]

Kévin Le Gouguec writes:

> I made patches for both maint and master because
>
> - the patch for maint has a small conflict in org-compat when applied
>   against master,
>
> - I'm not sure which branch will eventually be synchronized with
>   emacs-27 (assuming we'll include this fix in 27.2).

Thanks.  This patch is appropriate for maint in my view.

> diff --git a/lisp/org-compat.el b/lisp/org-compat.el
> index c757355ba..c0f4833a3 100644
> --- a/lisp/org-compat.el
> +++ b/lisp/org-compat.el
> @@ -101,6 +101,21 @@ is nil)."
>    (defun org-time-convert-to-list (time)
>      (seconds-to-time (float-time time))))
>  
> +(defmacro org--extended-face (attributes)
> +  "Make face that extends beyond end of line.
> +
> +Up to Emacs 26, all faces extended beyond end of line; getting
> +the same behaviour starting with Emacs 27 requires :extend t."
> +  `(nconc ,attributes (when (>= emacs-major-version 27) '(:extend t))))

Two minor thoughts, neither really important:

  * Style nit-pick: s/when/and/, as the return value is of interest.

  * I'm guessing your main reason for choosing a macro when a function
    would do here is to get inline expansion at compile time.

However ...

>  ;;; Emacs < 26.1 compatibility
>  
> diff --git a/lisp/org-faces.el b/lisp/org-faces.el
> index 30eab9bc6..4c5a51624 100644
> --- a/lisp/org-faces.el
> +++ b/lisp/org-faces.el
> @@ -393,7 +393,7 @@ follows a #+DATE:, #+AUTHOR: or #+EMAIL: keyword."
>    "Face for #+TITLE:, #+AUTHOR:, #+EMAIL: and #+DATE: keywords."
>    :group 'org-faces)
> -(defface org-block '((t :inherit shadow))
> +(defface org-block `((t ,(org--extended-face '(:inherit shadow))))
>    "Face text in #+begin ... #+end blocks.
>  For source-blocks `org-src-block-faces' takes precedence."
>    :group 'org-faces

... the main thing I wonder is whether this kludge is necessary at all.
AFAICT unconditionally including :extend in the face spec doesn't seem
to bother earlier Emacs versions.  Even if there is a reason to avoid
:extend on older versions, it's perhaps an overkill to add a
compatibility macro for just one spot.  Maybe instead just define it
directly in the value for org-block:

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


If org--extended-face stays, org-face.el should be adjusted to load
org-compat.el.  (`make single' flags this.)

Re: Bug#42184: org-fontify-whole-*-line in emacs 27

[Kévin Le Gouguec]

Kyle Meyer <kyle@kyleam.com> writes:

>> +(defmacro org--extended-face (attributes)
>> +  "Make face that extends beyond end of line.
>> +
>> +Up to Emacs 26, all faces extended beyond end of line; getting
>> +the same behaviour starting with Emacs 27 requires :extend t."
>> +  `(nconc ,attributes (when (>= emacs-major-version 27) '(:extend t))))
>
> Two minor thoughts, neither really important:
>
>   * Style nit-pick: s/when/and/, as the return value is of interest.

OK; I didn't know 'when' had a "for side-effect only" connotation, I was
using it as a shorthand for (if COND FORM nil).

> ... the main thing I wonder is whether this kludge is necessary at all.
> AFAICT unconditionally including :extend in the face spec doesn't seem
> to bother earlier Emacs versions.

Huh.  Based on the discussion for bug#37774[1][2][3][4], I had assumed
this kind of kludge would be necessary, but both Emacs 25.3 and 26.3
seem to evaluate and byte-compile the following snippet with no errors:

#+begin_src elisp
(defface foobar '((t (:extend t)))
  "Test extend in 26.3"
  :group 'org-faces)

(custom-set-faces
 '(foobar ((t (:extend nil))) t))
#+end_src

Obviously I'm all for removing this shim if it's not needed.  From some
light testing it looks like removing it breaks the customization widget,
though?

I'll post a revised patch as soon as someone can confirm or refute my
observations.

[1] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37774#41
[2] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37774#53
[3] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37774#71
[4] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37774#80

>                                    Even if there is a reason to avoid
> :extend on older versions, it's perhaps an overkill to add a
> compatibility macro for just one spot.

Right, that macro dates from an earlier patch, where I unconditionally
set :extend t on a bunch of faces[5].  I agree that it's overkill for
just org-block.

[5] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=42184#17

> If org--extended-face stays, org-face.el should be adjusted to load
> org-compat.el.  (`make single' flags this.)

(Ugh, I actually got that right in earlier patches.)


Thanks for the review.  As I said, I'll post an updated patch as soon as
someone confirms or refutes my impression that :extend messes with the
Customize widget for Emacs <27.

Re: Bug#42184: org-fontify-whole-*-line in emacs 27

[Kyle Meyer]

Kévin Le Gouguec writes:

> Huh.  Based on the discussion for bug#37774[1][2][3][4], I had assumed
> this kind of kludge would be necessary, but both Emacs 25.3 and 26.3
> seem to evaluate and byte-compile the following snippet with no errors:
>
> #+begin_src elisp
> (defface foobar '((t (:extend t)))
>   "Test extend in 26.3"
>   :group 'org-faces)
>
> (custom-set-faces
>  '(foobar ((t (:extend nil))) t))
> #+end_src
>
> Obviously I'm all for removing this shim if it's not needed.  From some
> light testing it looks like removing it breaks the customization widget,
> though?

Ah, good point.  I didn't think to check that in my testing, and indeed
it looks broken.  Okay, let's go with

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

as the org-block spec then.

Thanks.

Re: Bug#42184: org-fontify-whole-*-line in emacs 27

[Kévin Le Gouguec]

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

Kyle Meyer <kyle@kyleam.com> writes:

>                   Okay, let's go with
>
>   `((t :inherit shadow ,@(and (>= emacs-major-version 27) '(:extend t))))
>
> as the org-block spec then.

Done; patch attached.


[-- Attachment #2: 0001-Fix-org-fontify-whole-line-by-setting-face-extension.patch --]
[-- Type: text/x-patch, Size: 2792 bytes --]

From 41ab9f8f0c2f02f1951a412265b01a9ac5fa5a96 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?K=C3=A9vin=20Le=20Gouguec?= <kevin.legouguec@gmail.com>
Date: Fri, 7 Aug 2020 11:04:53 +0200
Subject: [PATCH] Fix org-fontify-whole-*-line by setting face extension
 (bug#42184)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* lisp/org-faces.el (org-block): Set background extension beyond
end-of-line.
* lisp/org-compat.el (org--set-faces-extend): New function to
temporarily (re)set :extend for Emacs≥27.
* lisp/org.el (org-mode): Call it to set the extend attribute of
relevant faces to the correct value.
---
 lisp/org-compat.el | 8 ++++++++
 lisp/org-faces.el  | 3 ++-
 lisp/org.el        | 6 +++++-
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/lisp/org-compat.el b/lisp/org-compat.el
index c757355ba..e405df0b5 100644
--- a/lisp/org-compat.el
+++ b/lisp/org-compat.el
@@ -101,6 +101,14 @@ is nil)."
   (defun org-time-convert-to-list (time)
     (seconds-to-time (float-time time))))
 
+(defun org--set-faces-extend (faces extend-p)
+  "Set the :extend attribute of FACES to EXTEND-P.
+
+This is a no-op for Emacs versions lower than 27, since face
+extension beyond end of line was not controllable."
+  (when (fboundp 'set-face-extend)
+    (mapc (lambda (f) (set-face-extend f extend-p)) faces)))
+
 \f
 ;;; Emacs < 26.1 compatibility
 
diff --git a/lisp/org-faces.el b/lisp/org-faces.el
index 30eab9bc6..ff2b0e611 100644
--- a/lisp/org-faces.el
+++ b/lisp/org-faces.el
@@ -393,7 +393,8 @@ follows a #+DATE:, #+AUTHOR: or #+EMAIL: keyword."
   "Face for #+TITLE:, #+AUTHOR:, #+EMAIL: and #+DATE: keywords."
   :group 'org-faces)
 
-(defface org-block '((t :inherit shadow))
+(defface org-block `((t :inherit shadow
+			,@(and (>= emacs-major-version 27) '(:extend t))))
   "Face text in #+begin ... #+end blocks.
 For source-blocks `org-src-block-faces' takes precedence."
   :group 'org-faces
diff --git a/lisp/org.el b/lisp/org.el
index 007dd6e2a..34c0235c1 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -4940,7 +4940,11 @@ The following commands are available:
   ;; Try to set `org-hide' face correctly.
   (let ((foreground (org-find-invisible-foreground)))
     (when foreground
-      (set-face-foreground 'org-hide foreground))))
+      (set-face-foreground 'org-hide foreground)))
+  ;; Set face extension as requested.
+  (org--set-faces-extend '(org-block-begin-line org-block-end-line)
+                         org-fontify-whole-block-delimiter-line)
+  (org--set-faces-extend org-level-faces org-fontify-whole-heading-line))
 
 ;; Update `customize-package-emacs-version-alist'
 (add-to-list 'customize-package-emacs-version-alist
-- 
2.28.0


[-- Attachment #3: Type: text/plain, Size: 42 bytes --]


> Thanks.

Again, thanks for the review!

Re: Bug#42184: org-fontify-whole-*-line in emacs 27

[Kyle Meyer]

Kévin Le Gouguec writes:

> Kyle Meyer <kyle@kyleam.com> writes:
>
>>                   Okay, let's go with
>>
>>   `((t :inherit shadow ,@(and (>= emacs-major-version 27) '(:extend t))))
>>
>> as the org-block spec then.
>
> Done; patch attached.

Applied (81e294847).

Bug#42184: org-fontify-whole-*-line in emacs 27 (was: Couple of issues with org block meta lines faces)

[Kévin Le Gouguec]

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

(Apologies for the possible duplicate; I tried to send this with Gmane
2 hours ago and AFAICT it hasn't made it through.  Retrying over SMTP…)

-------------------- Start of forwarded message --------------------
From: Kévin Le Gouguec <kevin.legouguec@gmail.com>
Subject: Bug#42184: org-fontify-whole-*-line in emacs 27 (was: Couple of issues with org block meta lines faces)
Date: Tue, 14 Jul 2020 12:19:38 +0200


[-- Attachment #2.1: Type: text/plain, Size: 911 bytes --]

Could Org maintainers weigh in on bug#42184?

To recap my understanding of the issue:

- org-fontify-whole-heading-line controls whether
  org-set-font-lock-defaults includes the final newline in the headline
  regexp (resp. org-fontify-whole-block-delimiter-line with begin/end
  block regexps).

- This assumes that fontifying the final newline is enough to fontify
  everything beyond this newline.

- This assumption is no longer valid with Emacs 27, where this extension
  is opt-in, using the :extend face attribute.

With Eli's help, I proposed a patch for org-mode against the emacs-27
branch that does something similar to what is done for the org-hide
face: when setting up the major mode, depending on those user options,
the :extend attribute is (re)set for the relevant faces (using a
compatibility function defined in org-compat).

I've reattached the patch for convenience.  Does it look sound?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2.2: 0001-Fix-org-fontify-whole-line-by-setting-face-extension.patch --]
[-- Type: text/x-diff, Size: 1853 bytes --]

> From 07d123c548051eb7f6bbac5c7f5a4e4b8411f976 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?K=C3=A9vin=20Le=20Gouguec?= <kevin.legouguec@gmail.com>
Date: Thu, 9 Jul 2020 16:02:49 +0200
Subject: [PATCH] Fix org-fontify-whole-*-line by setting face extension
 (bug#42184)

* lisp/org/org-compat.el (org--set-faces-extend): New function to set
face extension, for Emacs versions where this attribute exists.
* lisp/org/org.el (org-mode): Call it to set the extend attribute of
relevant faces to the correct value.
---
 lisp/org/org-compat.el | 4 ++++
 lisp/org/org.el        | 6 +++++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/lisp/org/org-compat.el b/lisp/org/org-compat.el
index c1aaf17ca2..fcc325e475 100644
--- a/lisp/org/org-compat.el
+++ b/lisp/org/org-compat.el
@@ -101,6 +101,10 @@ org-table1-hline-regexp
   (defun org-time-convert-to-list (time)
     (seconds-to-time (float-time time))))
 
+(defun org--set-faces-extend (faces extend-p)
+  (when (fboundp 'set-face-extend)
+    (mapc (lambda (f) (set-face-extend f extend-p)) faces)))
+
 \f
 ;;; Emacs < 26.1 compatibility
 
diff --git a/lisp/org/org.el b/lisp/org/org.el
index 568f5b9b87..fb31336ea4 100644
--- a/lisp/org/org.el
+++ b/lisp/org/org.el
@@ -4944,7 +4944,11 @@ org-mode
   ;; Try to set `org-hide' face correctly.
   (let ((foreground (org-find-invisible-foreground)))
     (when foreground
-      (set-face-foreground 'org-hide foreground))))
+      (set-face-foreground 'org-hide foreground)))
+  ;; Set face extension as requested.
+  (org--set-faces-extend '(org-block-begin-line org-block-end-line)
+                         org-fontify-whole-block-delimiter-line)
+  (org--set-faces-extend org-level-faces org-fontify-whole-heading-line))
 
 ;; Update `customize-package-emacs-version-alist'
 (add-to-list 'customize-package-emacs-version-alist
-- 
2.27.0


[-- Attachment #2.3: Type: text/plain, Size: 233 bytes --]



In addition to this issue, I'd like to ask about the org-block face:
should it be defined with :extend t unconditionally?  As explained in
bug#42184#35, I feel like it should (cf. attached screenshot), but
that's only my opinion.


[-- Attachment #2.4: org-block.png --]
[-- Type: image/png, Size: 319121 bytes --]

[-- Attachment #3: Type: text/plain, Size: 67 bytes --]

-------------------- End of forwarded message --------------------