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

[Kévin Le Gouguec]

Ihor Radchenko <yantar92@posteo.net> writes:

> [sending to Org ML in-reply to the relevant thread]

[Thanks!]

> Kévin Le Gouguec <kevin.legouguec@gmail.com> writes:
>
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=57003
> 28.1.90; Can local variables be loaded before loading major mode?
>
>> … reminded me of a patch I submitted to the Org ML… some time ago 😣
>> (sorry for not following up) to set TODO keywords via .dir-locals.el:
>>
>> https://list.orgmode.org/87a70stkmv.fsf@gmail.com/
>
> Your patch is not listed on https://updates.orgmode.org/
> It is also not in my records (I am only following patches closely since
> the beginning of this year).
> So, it slipped through the cracks.

(Right, that's entirely on me, I posted it knowing that an Org release
was pending and figuring I'd come back later…  well, better late than
never)

>> * could do so piecemeal, adding support for variables one at a time as
>>   people chime in the ML to express a need.
>
>> E.g. my patch only added support for org-todo-keywords and
>> org-todo-keyword-faces, but it laid the foundation for adding support
>> for other variables later.
>
> I'd prefer to solve it once and for all. I tried early loading of
> file-local variables in the past, but had to revert the commit because
> of major issues. See
> https://list.orgmode.org/87r11wkmew.fsf@ucl.ac.uk/T/#mab6359ed2107d5515c6bb6b266551f0c5049ceca
>
> Maybe the hook approach can work better. But I'd prefer to discuss all
> the possible caveats first.

My reasoning for keeping the current initialization code untouched and
_re_computing stuff in hack-local-variables-hook hinged on…

* refactoring being fraught; since Org already has a "blessed" way to do
  more or less what file/dir-locals do (SETUPFILEs), I figured it wasn't
  worth rocking everyone's boat for the benefit of the few,

* the prior art in other markup modes (markdown-handle-local-variables,
  font-latex-after-hacking-local-variables).

>> Also to try to reduce the risk of breakage, it went for "compute Org
>> settings normally; then selectively recompute some if relevant variables
>> are found in dir/file-locals".  That way "regular" Org users who rely
>> rather on SETUPFILEs wouldn't be impacted, only "early adopters" of
>> dir/file-locals might shoot themselves in the foot.
>
> I am not sure what is the problem with SETUPFILE.
> We can simply load it in the hook. 

I wasn't suggesting there's a problem with SETUPFILEs; my point was that
I considered two categories of users:

* those who are happy with SETUPFILEs: my implementation goal was to
  "guarantee" that my patch would have zero impact on them,

* those who want to play with dir/file-locals (👋): conversely, I wanted
  to make sure that only them would get to "pick up the pieces" when
  something would inevitably break.

This patch might have been my first foray into Org's init code, so it
felt too risky to go with any approach other than "keep the
implementation for the established features _exactly_ _as_ _now_; stuff
all the experimental stuff in hack-local-variables-hook".

>                                    Though the priority of SETUPFILE vs.
> local variables should be discussed. Probably, local variables should
> take precedence to keep things consistent with the rest of Emacs.

No strong opinions there.  I'm not even sure how my patch handled that?
🤔

> +(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)))

IIUC the logic goes org-set-regexps-and-options ⇒ org-collect-keywords ⇒
org--collect-keywords-1, and that's where SETUPFILE is processed?  So
currently the SETUPFILE would have priority 🙃 (unless I'm misreading
the code)