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

[Kévin Le Gouguec]

Kévin Le Gouguec <address@hidden> writes:

> Can anyone confirm that this would (in principle) be the way forward, or
> tell me if I am missing something[3]?

I went ahead and cooked up a proof-of-concept patch, which

(1) adds safe-local-variable properties to org-todo-keywords and
    org-todo-keyword-faces,

(2) stops applying default-value to org-todo-keywords,

(3) delays regexps/font-lock setups until after file- and dir-local
    variables have been set.

While this patch contains a few things that make me weary[1], it solves
my use-case, and passes the current test suite with Emacs 26.3 and 28.

Does this look sound overall?  Does anyone have any idea what kind of
breakage might be slipping through the test suite?

Thank you for your time.


[1] - It's hard to feel confident that moving org-regexps-and-options
      and org-set-font-lock-defaults like this will not introduce
      regressions.

    - Also, these safe-local-variable validation functions could
      probably use some unit tests.


diff --git a/lisp/org-faces.el b/lisp/org-faces.el
index d78b606ec..544563276 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)
+             (pcase pair
+               (`(,keyword . ,face)
+                (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..7f4672058 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)
@@ -4801,8 +4806,6 @@ The following commands are available:
      (vconcat (mapcar (lambda (c) (make-glyph-code c 'org-ellipsis))
                      org-ellipsis)))
     (setq buffer-display-table org-display-table))
-  (org-set-regexps-and-options)
-  (org-set-font-lock-defaults)
   (when (and org-tag-faces (not org-tags-special-faces-re))
     ;; tag faces set outside customize.... force initialization.
     (org-set-tag-faces 'org-tag-faces org-tag-faces))
@@ -4909,7 +4912,16 @@ 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)))
+
+  ;; For file-visiting buffers, delay some setup until after
+  ;; file-local and directory-local variables have been set.
+  (if (buffer-file-name)
+      (progn
+       (add-hook 'hack-local-variables-hook 'org-set-regexps-and-options 1 t)
+       (add-hook 'hack-local-variables-hook 'org-set-font-lock-defaults 1 t))
+    (org-set-regexps-and-options)
+    (org-set-font-lock-defaults)))
 
 ;; Update `customize-package-emacs-version-alist'
 (add-to-list 'customize-package-emacs-version-alist