Re: [O] [bug?] org-copy-face doesn’t add faces to org-faces customize group
Hi Aaron, Aaron Ecay wrote: 2014ko irailak 23an, Sebastien Vauban-ek idatzi zuen: OK, thanks for the followup. I’m attaching the patch to this email. If I don’t hear any further feedback, I’ll commit it to master in a few days. I'd say: commit it now, so that it really gets to a broad audience. Anyway, would there be problems, they would be very, very limited: face problems, that's all! The patch is now pushed to master. That works great. Thanks. Best regards, Seb -- Sebastien Vauban
Re: [O] [bug?] org-copy-face doesn’t add faces to org-faces customize group
Hi Seb, 2014ko irailak 23an, Sebastien Vauban-ek idatzi zuen: OK, thanks for the followup. I’m attaching the patch to this email. If I don’t hear any further feedback, I’ll commit it to master in a few days. I'd say: commit it now, so that it really gets to a broad audience. Anyway, would there be problems, they would be very, very limited: face problems, that's all! The patch is now pushed to master. Thanks, -- Aaron Ecay
Re: [O] [bug?] org-copy-face doesn’t add faces to org-faces customize group
Hi Aaron, Aaron Ecay wrote: 2014ko abuztuak 29an, Sebastien Vauban-ek idatzi zuen: I think it's related to an Emacs bug (#16440) which I reported on the Org mailing list in February. I don’t completely understand what is going on in that bug report. My proposal is to convert org-copy-face to defface. I think this is the right thing (TM). I can’t tell if this would fix your problem or not, but if it’s exclusive to the faces I listed in my previous email the answer is probably “yes.” From what I can see, yes, the faces which are wrongly displayed (before re-applying the theme) match the ones defined by `org-copy-face'. So, this seems the right way to go. Best regards, Seb -- Sebastien Vauban
Re: [O] [bug?] org-copy-face doesn’t add faces to org-faces customize group
Hi Seb, 2014ko irailak 23an, Sebastien Vauban-ek idatzi zuen: Hi Aaron, Aaron Ecay wrote: 2014ko abuztuak 29an, Sebastien Vauban-ek idatzi zuen: I think it's related to an Emacs bug (#16440) which I reported on the Org mailing list in February. I don’t completely understand what is going on in that bug report. My proposal is to convert org-copy-face to defface. I think this is the right thing (TM). I can’t tell if this would fix your problem or not, but if it’s exclusive to the faces I listed in my previous email the answer is probably “yes.” From what I can see, yes, the faces which are wrongly displayed (before re-applying the theme) match the ones defined by `org-copy-face'. So, this seems the right way to go. OK, thanks for the followup. I’m attaching the patch to this email. If I don’t hear any further feedback, I’ll commit it to master in a few days. From 128726a88ca2ec2232071cd9a6d7869df01fd953 Mon Sep 17 00:00:00 2001 From: Aaron Ecay aarone...@gmail.com Date: Tue, 23 Sep 2014 13:21:22 -0400 Subject: [PATCH] org-faces: remove org-copy-face MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * lisp/org-faces.el (org-copy-face): Remove function. (org-checkbox-statistics-todo, org-checkbox-statistics-done) (org-block-begin-line, org-block-end-line, org-quote) (org-verse, org-agenda-date, org-agenda-date-today) (org-agenda-clocking, org-agenda-date-weekend) (org-agenda-current-time, org-mode-line-clock) (org-mode-line-clock-overrun): Convert to `defface' from `org-copy-face'. The ‘org-copy-face’ function didn’t properly deal with face customizations and color themes. --- lisp/org-faces.el | 96 +++ 1 file changed, 54 insertions(+), 42 deletions(-) diff --git a/lisp/org-faces.el b/lisp/org-faces.el index 6e62ad0..36f810e 100644 --- a/lisp/org-faces.el +++ b/lisp/org-faces.el @@ -31,19 +31,6 @@ (require 'org-macs) (require 'org-compat) -(defun org-copy-face (old-face new-face docstring rest attributes) - (unless (facep new-face) -(if (fboundp 'set-face-attribute) - (progn - (make-face new-face) - (set-face-attribute new-face nil :inherit old-face) - (apply 'set-face-attribute new-face nil attributes) - (set-face-doc-string new-face docstring)) - (copy-face old-face new-face) - (if (fboundp 'set-face-doc-string) - (set-face-doc-string new-face docstring) -(put 'org-copy-face 'lisp-indent-function 2) - (when (featurep 'xemacs) (put 'mode-line 'face-alias 'modeline)) @@ -427,12 +414,15 @@ determines if it is a foreground or a background color. Face for checkboxes. :group 'org-faces) +(defface org-checkbox-statistics-todo + '((t (:inherit org-todo))) + Face used for unfinished checkbox statistics. + :group 'org-faces) -(org-copy-face 'org-todo 'org-checkbox-statistics-todo - Face used for unfinished checkbox statistics.) - -(org-copy-face 'org-done 'org-checkbox-statistics-done - Face used for finished checkbox statistics.) +(defface org-checkbox-statistics-done + '((t (:inherit org-done))) + Face used for finished checkbox statistics. + :group 'org-faces) (defcustom org-tag-faces nil Faces for specific tags. @@ -537,11 +527,15 @@ follows a #+DATE:, #+AUTHOR: or #+EMAIL: keyword. :group 'org-faces :version 22.1) -(org-copy-face 'org-meta-line 'org-block-begin-line - Face used for the line delimiting the begin of source blocks.) +(defface org-block-begin-line + '((t (:inherit org-meta-line))) + Face used for the line delimiting the begin of source blocks. + :group 'org-faces) -(org-copy-face 'org-meta-line 'org-block-end-line - Face used for the line delimiting the end of source blocks.) +(defface org-block-end-line + '((t (:inherit org-block-begin-line))) + Face used for the line delimiting the end of source blocks. + :group 'org-faces) (defface org-verbatim (org-compatible-face 'shadow @@ -557,10 +551,15 @@ follows a #+DATE:, #+AUTHOR: or #+EMAIL: keyword. :group 'org-faces :version 22.1) -(org-copy-face 'org-block 'org-quote - Face for #+BEGIN_QUOTE ... #+END_QUOTE blocks.) -(org-copy-face 'org-block 'org-verse - Face for #+BEGIN_VERSE ... #+END_VERSE blocks.) +(defface org-quote + '((t (:inherit org-block))) + Face for #+BEGIN_QUOTE ... #+END_QUOTE blocks. + :group 'org-faces) + +(defface org-verse + '((t (:inherit org-block))) + Face for #+BEGIN_VERSE ... #+END_VERSE blocks. + :group 'org-faces) (defcustom org-fontify-quote-and-verse-blocks nil Non-nil means, add a special face to #+begin_quote and #+begin_verse block. @@ -597,21 +596,28 @@ content of these blocks will still be treated as Org syntax. Face used in agenda for captions and dates. :group 'org-faces) -(org-copy-face 'org-agenda-structure 'org-agenda-date - Face used in agenda for normal days.) +(defface org-agenda-date + '((t (:inherit org-agenda-structure))) + Face used in agenda for normal days. +
Re: [O] [bug?] org-copy-face doesn’t add faces to org-faces customize group
Hi Aaron, Aaron Ecay wrote: 2014ko irailak 23an, Sebastien Vauban-ek idatzi zuen: Aaron Ecay wrote: 2014ko abuztuak 29an, Sebastien Vauban-ek idatzi zuen: I think it's related to an Emacs bug (#16440) which I reported on the Org mailing list in February. I don’t completely understand what is going on in that bug report. My proposal is to convert org-copy-face to defface. I think this is the right thing (TM). I can’t tell if this would fix your problem or not, but if it’s exclusive to the faces I listed in my previous email the answer is probably “yes.” From what I can see, yes, the faces which are wrongly displayed (before re-applying the theme) match the ones defined by `org-copy-face'. So, this seems the right way to go. OK, thanks for the followup. I’m attaching the patch to this email. If I don’t hear any further feedback, I’ll commit it to master in a few days. I'd say: commit it now, so that it really gets to a broad audience. Anyway, would there be problems, they would be very, very limited: face problems, that's all! Best regards, Seb -- Sebastien Vauban
Re: [O] [bug?] org-copy-face doesn’t add faces to org-faces customize group
Hi Seb, 2014ko abuztuak 29an, Sebastien Vauban-ek idatzi zuen: I think it's related to an Emacs bug (#16440) which I reported on the Org mailing list in February. I don’t completely understand what is going on in that bug report. My proposal is to convert org-copy-face to defface. I can’t tell if this would fix your problem or not, but if it’s exclusive to the faces I listed in my previous email the answer is probably “yes.” -- Aaron Ecay
Re: [O] [bug?] org-copy-face doesn’t add faces to org-faces customize group
Aaron Ecay wrote: I’ve noticed that the faces defined by org-copy-face are not added to the org-faces customize group. This is in accordance with the docstring of ‘copy-face’, which says (in part) “This function does not copy face customization data, so NEW-FACE will not be made customizable. Most Lisp code should not call this function; use `defface' with :inherit instead.” I think it’s at best an odd surprise and at worst a bug that all org’s faces are not accessible from the org-faces customize group. Would there be any objection to replacing all uses of this function with :inherit as recommended by the docstring, and removing the org-copy-face function? For reference, here are the uses of the function, as returned by rgrep: ./lisp/org-faces.el:431:(org-copy-face 'org-todo 'org-checkbox-statistics-todo ./lisp/org-faces.el:434:(org-copy-face 'org-done 'org-checkbox-statistics-done ./lisp/org-faces.el:540:(org-copy-face 'org-meta-line 'org-block-begin-line ./lisp/org-faces.el:543:(org-copy-face 'org-meta-line 'org-block-end-line ./lisp/org-faces.el:560:(org-copy-face 'org-block 'org-quote ./lisp/org-faces.el:562:(org-copy-face 'org-block 'org-verse ./lisp/org-faces.el:600:(org-copy-face 'org-agenda-structure 'org-agenda-date ./lisp/org-faces.el:603:(org-copy-face 'org-agenda-date 'org-agenda-date-today ./lisp/org-faces.el:607:(org-copy-face 'secondary-selection 'org-agenda-clocking ./lisp/org-faces.el:610:(org-copy-face 'org-agenda-date 'org-agenda-date-weekend ./lisp/org-faces.el:719:(org-copy-face 'org-time-grid 'org-agenda-current-time ./lisp/org-faces.el:791:(org-copy-face 'mode-line 'org-mode-line-clock ./lisp/org-faces.el:793:(org-copy-face 'mode-line 'org-mode-line-clock-overrun I think it's related to an Emacs bug (#16440) which I reported on the Org mailing list in February. As stated by Eli (in http://debbugs.gnu.org/cgi/bugreport.cgi?bug=16440+): ╭ │ Org uses org-copy-face to define the faces that you show in your │ screencast, and org-copy-face assumes the face it inherits from │ already exists. But loading a theme now doesn't create the faces, it │ only prepares the data for when the face will be created. So :inherit │ in org-copy-face doesn't do what you expect. │ │ I guess either some change is needed in how themes are handled, or │ org-copy-face needs to change to follow suit. (CC to Bastien for │ that.) ╰ In the same functional area, there is also the bug #15298 still pending (http://debbugs.gnu.org/cgi/bugreport.cgi?bug=15298) about Background color lost when highlighting a string. Best regards, Seb -- Sebastien Vauban
[O] [bug?] org-copy-face doesn’t add faces to org-faces customize group
Hello all, I’ve noticed that the faces defined by org-copy-face are not added to the org-faces customize group. This is in accordance with the docstring of ‘copy-face’, which says (in part) “This function does not copy face customization data, so NEW-FACE will not be made customizable. Most Lisp code should not call this function; use `defface' with :inherit instead.” I think it’s at best an odd surprise and at worst a bug that all org’s faces are not accessible from the org-faces customize group. Would there be any objection to replacing all uses of this function with :inherit as recommended by the docstring, and removing the org-copy-face function? For reference, here are the uses of the function, as returned by rgrep: ./lisp/org-faces.el:431:(org-copy-face 'org-todo 'org-checkbox-statistics-todo ./lisp/org-faces.el:434:(org-copy-face 'org-done 'org-checkbox-statistics-done ./lisp/org-faces.el:540:(org-copy-face 'org-meta-line 'org-block-begin-line ./lisp/org-faces.el:543:(org-copy-face 'org-meta-line 'org-block-end-line ./lisp/org-faces.el:560:(org-copy-face 'org-block 'org-quote ./lisp/org-faces.el:562:(org-copy-face 'org-block 'org-verse ./lisp/org-faces.el:600:(org-copy-face 'org-agenda-structure 'org-agenda-date ./lisp/org-faces.el:603:(org-copy-face 'org-agenda-date 'org-agenda-date-today ./lisp/org-faces.el:607:(org-copy-face 'secondary-selection 'org-agenda-clocking ./lisp/org-faces.el:610:(org-copy-face 'org-agenda-date 'org-agenda-date-weekend ./lisp/org-faces.el:719:(org-copy-face 'org-time-grid 'org-agenda-current-time ./lisp/org-faces.el:791:(org-copy-face 'mode-line 'org-mode-line-clock ./lisp/org-faces.el:793:(org-copy-face 'mode-line 'org-mode-line-clock-overrun Thanks, -- Aaron Ecay
