* [bug?] org-copy-face doesn’t add faces to org-faces customize group @ 2014-08-27 1:41 Aaron Ecay 2014-08-29 11:45 ` Sebastien Vauban 0 siblings, 1 reply; 8+ messages in thread From: Aaron Ecay @ 2014-08-27 1:41 UTC (permalink / raw) To: orgmode list 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [bug?] org-copy-face doesn’t add faces to org-faces customize group 2014-08-27 1:41 [bug?] org-copy-face doesn’t add faces to org-faces customize group Aaron Ecay @ 2014-08-29 11:45 ` Sebastien Vauban 2014-08-30 19:03 ` Aaron Ecay 0 siblings, 1 reply; 8+ messages in thread From: Sebastien Vauban @ 2014-08-29 11:45 UTC (permalink / raw) To: emacs-orgmode-mXXj517/zsQ 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [bug?] org-copy-face doesn’t add faces to org-faces customize group 2014-08-29 11:45 ` Sebastien Vauban @ 2014-08-30 19:03 ` Aaron Ecay [not found] ` <87wq9pssuo.fsf-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 0 siblings, 1 reply; 8+ messages in thread From: Aaron Ecay @ 2014-08-30 19:03 UTC (permalink / raw) To: Sebastien Vauban, emacs-orgmode 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <87wq9pssuo.fsf-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [bug?] org-copy-face doesn’t add faces to org-faces customize group [not found] ` <87wq9pssuo.fsf-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2014-09-23 9:40 ` Sebastien Vauban 2014-09-23 17:27 ` Aaron Ecay 0 siblings, 1 reply; 8+ messages in thread From: Sebastien Vauban @ 2014-09-23 9:40 UTC (permalink / raw) To: emacs-orgmode-mXXj517/zsQ 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [bug?] org-copy-face doesn’t add faces to org-faces customize group 2014-09-23 9:40 ` Sebastien Vauban @ 2014-09-23 17:27 ` Aaron Ecay 2014-09-23 18:00 ` Sebastien Vauban 0 siblings, 1 reply; 8+ messages in thread From: Aaron Ecay @ 2014-09-23 17:27 UTC (permalink / raw) To: Sebastien Vauban, emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 977 bytes --] 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. [-- Attachment #2: 0001-org-faces-remove-org-copy-face.patch --] [-- Type: text/x-diff, Size: 6400 bytes --] From 128726a88ca2ec2232071cd9a6d7869df01fd953 Mon Sep 17 00:00:00 2001 From: Aaron Ecay <aaronecay@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." + :group 'org-faces) -(org-copy-face 'org-agenda-date 'org-agenda-date-today +(defface org-agenda-date-today + '((t (:inherit org-agenda-date :weight bold :italic t))) "Face used in agenda for today." - :weight 'bold :italic 't) + :group 'org-faces) -(org-copy-face 'secondary-selection 'org-agenda-clocking - "Face marking the current clock item in the agenda.") +(defface org-agenda-clocking + '((t (:inherit secondary-selection))) + "Face marking the current clock item in the agenda." + :group 'org-faces) -(org-copy-face 'org-agenda-date 'org-agenda-date-weekend +(defface org-agenda-date-weekend + '((t (:inherit org-agenda-date :weight bold))) "Face used in agenda for weekend days. -See the variable `org-agenda-weekend-days' for a definition of which days -belong to the weekend." - :weight 'bold) + +See the variable `org-agenda-weekend-days' for a definition of +which days belong to the weekend." + :group 'org-faces) (defface org-scheduled (org-compatible-face nil @@ -716,8 +722,10 @@ month and 365.24 days for a year)." "Face used for time grids." :group 'org-faces) -(org-copy-face 'org-time-grid 'org-agenda-current-time - "Face used to show the current time in the time grid.") +(defface org-agenda-current-time + '((t (:inherit org-time-grid))) + "Face used to show the current time in the time grid." + :group 'org-faces) (defface org-agenda-diary (org-compatible-face 'default nil) @@ -788,11 +796,15 @@ level org-n-level-faces" :version "24.4" :package-version '(Org . "8.0")) -(org-copy-face 'mode-line 'org-mode-line-clock - "Face used for clock display in mode line.") -(org-copy-face 'mode-line 'org-mode-line-clock-overrun +(defface org-mode-line-clock + '((t (:inherit mode-line))) + "Face used for clock display in mode line." + :group 'org-faces) + +(defface org-mode-line-clock-overrun + '((t (:inherit mode-line :background "red"))) "Face used for clock display for overrun tasks in mode line." - :background "red") + :group 'org-faces) (provide 'org-faces) -- 2.1.0 [-- Attachment #3: Type: text/plain, Size: 15 bytes --] -- Aaron Ecay ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [bug?] org-copy-face doesn’t add faces to org-faces customize group 2014-09-23 17:27 ` Aaron Ecay @ 2014-09-23 18:00 ` Sebastien Vauban 2014-09-28 3:38 ` Aaron Ecay 0 siblings, 1 reply; 8+ messages in thread From: Sebastien Vauban @ 2014-09-23 18:00 UTC (permalink / raw) To: emacs-orgmode-mXXj517/zsQ 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [bug?] org-copy-face doesn’t add faces to org-faces customize group 2014-09-23 18:00 ` Sebastien Vauban @ 2014-09-28 3:38 ` Aaron Ecay 2014-09-29 10:06 ` Sebastien Vauban 0 siblings, 1 reply; 8+ messages in thread From: Aaron Ecay @ 2014-09-28 3:38 UTC (permalink / raw) To: Sebastien Vauban, emacs-orgmode 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [bug?] org-copy-face doesn’t add faces to org-faces customize group 2014-09-28 3:38 ` Aaron Ecay @ 2014-09-29 10:06 ` Sebastien Vauban 0 siblings, 0 replies; 8+ messages in thread From: Sebastien Vauban @ 2014-09-29 10:06 UTC (permalink / raw) To: emacs-orgmode-mXXj517/zsQ 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-09-29 10:06 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-08-27 1:41 [bug?] org-copy-face doesn’t add faces to org-faces customize group Aaron Ecay
2014-08-29 11:45 ` Sebastien Vauban
2014-08-30 19:03 ` Aaron Ecay
[not found] ` <87wq9pssuo.fsf-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2014-09-23 9:40 ` Sebastien Vauban
2014-09-23 17:27 ` Aaron Ecay
2014-09-23 18:00 ` Sebastien Vauban
2014-09-28 3:38 ` Aaron Ecay
2014-09-29 10:06 ` Sebastien Vauban
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).