Re: Setting org-todo-keywords through directory-local variables

[Kévin Le Gouguec]

Nicolas Goaziou <address@hidden> writes:

> This looks hackish.

Any bit in particular?  AFAICT hack-local-variables-hook is the expected
way to perform plumbing that might be affected by file/directory-local
variables.  It is used by e.g. shell-script-mode, cc-mode, markdown-mode
and AUCTeX, to name a few.  The docstring says:

> Major modes can use this to examine user-specified local variables
> in order to initialize other data structure based on them.

I think the buffer-file-name bit looks dodgy; I mainly did that to get
the unit tests to pass on this POC.

>                     Also, Org needs the STARTUP part early on, so you
> cannot really delay it.
>
> I /think/ the rest can be delayed, but I admit I'm not too sure either.

Right.  Now that I've looked at other major modes (especially
AUCTeX[1]), it seems a more conventional approach would be to

- keep the calls to org-set-regexps-and-options and
  org-set-font-lock-defaults where they are now,

- in hack-local-variables-hook, *if* file-local-variables-alist contains
  Org variables that affect those functions, and call them again to
  refresh regexps and fontification.

IIUC this would pretty much guarantee that things can only break for
weirdos like me who try to use directory-local variables.

> I think the expected way to do this is to add a SETUPFILE.

Thanks for the pointer!  Unless I'm misreading (info "(org) In-buffer
Settings"), I could move my SEQ_TODO settings there, but that wouldn't
work for org-todo-keyword-faces, right?


In light of your comments, and based on what I've seen in AUCTeX, I'm
attaching what I believe to be a less intrusive patch.  WDYT?


Thank you for taking the time to review this.  I'm not opposed to using
SETUPFILE (if I can handle org-todo-keyword-faces there); I'm just
wondering if this could be one more opportunity to have Org play nice
with other Emacs facilities (directory-local variables).


[1] 
https://git.savannah.gnu.org/cgit/auctex.git/tree/font-latex.el?h=release_12_2#n1435


diff --git a/lisp/org-faces.el b/lisp/org-faces.el
index d78b606ec..fc834f37d 100644
--- a/lisp/org-faces.el
+++ b/lisp/org-faces.el
@@ -291,7 +291,15 @@ determines if it is a foreground or a background color."
           (string :tag "Keyword")
           (choice :tag "Face   "
                   (string :tag "Color")
-                  (sexp :tag "Face")))))
+                  (sexp :tag "Face"))))
+  :safe (lambda (x)
+          (cl-every
+           (lambda (pair)
+            (let ((keyword (car pair))
+                  (face (cdr pair)))
+              (and (stringp keyword)
+                   (or (facep face) (listp face)))))
+           x)))
 
 (defface org-priority '((t :inherit font-lock-keyword-face))
   "Face used for priority cookies."
diff --git a/lisp/org.el b/lisp/org.el
index e577dc661..da38beb45 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -1945,7 +1945,13 @@ taken from the (otherwise obsolete) variable 
`org-todo-interpretation'."
                                         org-todo-interpretation-widgets))
                      widget))
                   (repeat
-                   (string :tag "Keyword"))))))
+                   (string :tag "Keyword")))))
+  :safe (lambda (x)
+          (cl-every
+           (lambda (seq)
+             (and (memq (car seq) '(sequence type))
+                  (cl-every (lambda (i) (stringp i)) (cdr seq))))
+           x)))
 
 (defvar-local org-todo-keywords-1 nil
   "All TODO and DONE keywords active in a buffer.")
@@ -4358,10 +4364,9 @@ related expressions."
                                     (cons 'sequence (split-string value)))
                                   (append (cdr (assoc "TODO" alist))
                                           (cdr (assoc "SEQ_TODO" alist)))))
-                  (let ((d (default-value 'org-todo-keywords)))
-                    (if (not (stringp (car d))) d
-                      ;; XXX: Backward compatibility code.
-                      (list (cons org-todo-interpretation d)))))))
+                  (if (not (stringp (car org-todo-keywords))) org-todo-keywords
+                    ;; XXX: Backward compatibility code.
+                    (list (cons org-todo-interpretation org-todo-keywords))))))
          (dolist (sequence todo-sequences)
            (let* ((sequence (or (run-hook-with-args-until-success
                                  'org-todo-setup-filter-hook sequence)
@@ -4909,7 +4914,18 @@ 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)))
+
+  (add-hook 'hack-local-variables-hook #'org--process-local-variables nil t))
+
+(defun org--process-local-variables ()
+  "Refresh settings affected by file-local or directory-local variables."
+  (when
+      (let ((local-vars (mapcar #'car file-local-variables-alist)))
+       (or (memq 'org-todo-keywords local-vars)
+           (memq 'org-todo-keyword-faces local-vars)))
+    (org-set-regexps-and-options)
+    (org-set-font-lock-defaults)))
 
 ;; Update `customize-package-emacs-version-alist'
 (add-to-list 'customize-package-emacs-version-alist