Wrap filetype detection hooks in module to allow order of evaluation specification with ModuleLoaded hook

[pylasnier]

I propose wrapping filetype detection config code in a dedicated module, for now I've named detect-<filetype>, which gets immediately required after defining it, so as to preserve the normal autoload behaviour for the filetype rcs.

What this change does is enable people to manage conflicts between their own plugin or kakrc code, without having to modify or delete default rcs from their autoload, by forcing their code to run after the filetype detection codes using the ModuleLoaded hook. This specifies an order of execution, and is consistent regardless of the order of loading of files, as per the behaviour described in #4841.

For example, the SML filetype detection regex may be partially overridden by making a new BufCreate hook, which runs after the default SML detection:

hook global ModuleLoaded detect-sml %{

hook global BufCreate .*Script\.sml %{
    set-option buffer filetype holscript
}

}

The changes I have included for now only address the issues from this discussion, but I propose similar changes may be added to all filetype rcs, or possibly any eagerly-evaluated autoload code which might be helpful to specify order of evaluation for.

This change could help solve the following issues:

• [REQUEST] A better way to prevent standard library plugins from loading #4375
• https://discuss.kakoune.com/t/how-do-you-easily-disable-the-default-init-file-for-a-filetype/2607
• https://discuss.kakoune.com/t/how-can-i-make-pepe-janet-kak-properly-override-built-in-janet-kak/1889

[krobelus]

I haven't had time to look at this in detail but
this might be a workaround that goes in the wrong direction.

In general I'd either fix the stdlib rc file, or use a different filetype
that doesn't clash. The stdlib files are supposed to work for almost everyone.

If your plugin disables it, that might break other use cases and plugins.

I’m working on an extension for a proof assistant called HOL4, which uses
script files written in SML, typically with the file format nameScript.sml

So the issue is that for your case, "*.sml" files should not have the sml filetype
but the holscript one?
Can we maybe detect these cases and build them into sml.kak?

Of course the user should always be in control;
it's probably possible to achieve your goal today
by being careful about hook ordering:
if your BufCreate hook is added after the stdlib's,
then the final filetype will be holscript (
and the transient sml filetype shouldn't matter, right?)

[pylasnier]

So the issue is that for your case, "*.sml" files should not have the sml filetype
but the holscript one?
Can we maybe detect these cases and build them into sml.kak?

I would rather keep the HOL4-specific stuff separate because it is not commonly used compared to SML, and I would rather not surprise a kak user who happened to decide to call their file somethingScript.sml. I would prefer it to be an opt-in plugin, especially because full functionality of the plugin depends on a HOL4 installation.

If your plugin disables it, that might break other use cases and plugins.

With this version of config, the HOL4 plugin doesn't disable SML detection, but merely takes precedence over it by setting the regex hook after. It should not break other use-cases, so long as the user does not name their file somethingScript.sml (which, by having the plugin, they have opted-in to).

As far as I can tell, this proposal of wrapping with a module should not impact default behaviour in any way. It simply sets a checkpoint which plugin or user code can wait for to allow their code to run after, which should be a helpful feature. This is the best solution I could come up with that uses existing syntax.

Of course the user should always be in control;
it's probably possible to achieve your goal today
by being careful about hook ordering:
if your BufCreate hook is added after the stdlib's,
then the final filetype will be holscript (
and the transient sml filetype shouldn't matter, right?)

This is exactly what my example does, it just uses the module wrapping and ModuleLoaded hook to achieve that, rather than modifying stdlib.

The point is that it is difficult to be careful about hook ordering. Many of these issues arise from the fact that stdlib is autoloaded after people's custom hooks and overrides them. I expect an idiomatic solution would be the allow the user to specify an order of execution, in spirit of the lazy, module-based evaluation that does not require file-level changes.

[pylasnier]

I have added similar detect-filetype modules to all filetypes, so ModuleLoaded hooks can be used to specify order of evaluation of detection and initialisation hooks for any filetype.

I really think a solution like this would be a useful addition to the rcs. Even if it's not this particular solution and its something else, I think it would be a step in the right direction to have some sort of feature which allows plugins to resolve conflicts with the stdlib without requiring the user to modify their default autoload. Perhaps some change to hooks.

[krobelus]

I wonder if we should change module names. With this PR, we have.

provide-module detect-apl %{
	hook global WinSetOption filetype=apl %{
	    require-module apl
	}
	...
}
provide-module apl %{
	add-highlighter shared/apl regions
	...
}

It's possible that we want to include the "filetype" path component,
to prevent future clashes.
We could do this in a backwards-compatible way
by also providing an empty module with the old name.

provide-module filetype-apl-detection %{
	hook global WinSetOption filetype=apl %{
	    require-module filetype-apl
	}
	...
}
provide-module filetype-apl %{
	provide-module apl %{} # historical
	add-highlighter shared/apl regions
	...
}

Not yet sure if this is worth it, but it seems like now is a good time to think about this.
There's no existing discussion in #2782.

I wonder what are the odds that our modules (like jump, make, tmux, screen) will ever collide with filetype modules of the same name..

[pylasnier]

I would be happy to use different names for the detection modules if you'd like. I would probably need to check but I'm not entirely sure if your suggested filetype module name change, though, wouldn't be breaking? The apl module would be kept hidden until filetype-apl is required?

Perhaps we could open an issue on the topic, I would personally consider it a separate change.

[krobelus]

huh, so this is an odd module where the filename (and thus the "detect-exherbo" hook)
don't match the actual filetype modules (exheres, paludis etc.).
I guess if we really qualify module names, this would imply
module names like filetype-exherbo-exheres and filetype-exherbo-paludis.
Moving all into a single module would probably work too, but I guess it's nice to allow multiple modules per file.

[pylasnier]

Oops, it indeed seems like I missed some hooks scattered around in files, I think I have cleaned everything up now.

I have opted to go with one detection module per file,

1. for consistency; some files (for example git.kak) have I think too many filetypes to want to split apart, so if those stay grouped then others should stay grouped
2. I believe users are unlikely to need these modules below the granularity of a file, since they exist only to specify an order of execution. They would simply want to state 'run the hooks from this file first', and I expect further specificity of hook ordering is redundant.
3. IMO slightly easier discovery, it's just the name of the kak file

[krobelus]

Thanks for this, this seems like the ideal solution to define custom
filetype detection hooks after the ones defined by the stdlib,
which is usually the right thing to do (as opposed to completely removing the stdlib one).
I agree that hook groups wouldn't help here.

Your last comment on Discourse was also helpful.

I guess this means that all scripts in "rc/filetype/*.kak" put all executable code into the "detect-$filetype" and "$filetype" modules?

Two possible future directions:

• for people who want to completely override a "$filetype" module, we
  can add provide-module -override as suggested in [REQUEST] A better way to prevent standard library plugins from loading #4375 (comment)
• for people who want to (also) override the filetype detection hooks,
  that is, either of these:
  • hook global BufCreate .*\.(sml|fun|sig)
  • hook global WinSetOption filetype=sml
  • hook -group sml-highlight global WinSetOption filetype=sml
    we could
  • either add hook groups to the first two, so they can use the familiar remove-hooks and set-option global disabled_hooks
  • or
    • move the "require-module detect-$foo" hooks into a "hook -once global KakBegin .*".
    • then they can use provide-module -override detect-sml %{}

[pylasnier]

I guess this means that all scripts in "rc/filetype/*.kak" put all executable code into the "detect-$filetype" and "$filetype" modules?

Almost everything, there are some hidden commands (like indentation commands) which I thought were probably redundant to wrap up. All hooks are in modules now.

[mawww]

Hi, coming late to this discussion, I am still unclear on how this is supposed to work, as far as I understand the only difference this introduces is that we will get a ModuleLoaded hook triggered when sourcing those filetype support files.

ModuleLoaded triggers immediately when we load those autoload files, but we do have special code so that adding a ModuleLoaded hook will trigger immediately if that module has already been loaded, which means that all ModuleLoaded hooks taking advantage of this in user configuration are equivalent to just running the commands they contain directly.

in other words, AFAIU, the only difference between:

hook global ModuleLoaded detect-sml %{
hook global BufCreate .*Script\.sml %{ set-option buffer filetype holscript }
}

and

hook global BufCreate .*Script\.sml %{  set-option buffer filetype holscript }

In the user kakrc, is that the former will only add the hook if we have sourced sml.kak while the latter will always add that hook.

Is my understanding correct ? Is that what you are trying to achieve with this change ?

[pylasnier]

The issue is that, as is commonly the case, hook global BufCreate .*Script\.sml %{ set-option buffer filetype holscript } gets run before the stdlib rcs from the autoload, so the hook gets shadowed. This change allows users to force their code to run after any particular stdlib filetype rc, so that their hooks can't get shadowed.

[mawww]

Hum, do you mean then that this hook is inside another autoload file ? Because if it is in the user kakrc it should run after.

The existing workaround for this is to use the KakBegin hook which should run after all autoload and user kakrc have been sourced. It is not a very elegant solution but I am not sure the problem you are trying to solve here is common enough to warrant the extra confusion introduced by the "immediately loaded module" pattern.

[pylasnier]

Oh I see, that could work to solve my issue. I hadn't come across that workaround before.

This change could still offer solutions that the KakBegin hook workaround can't, for example if the order of evaluation issue comes up twice in a chain. And I would argue that, because this change doesn't change existing behaviour, it might still be valuable for the sake of including more stdlib code into patterns of lazy evaluation. But if you think it would be confusing that's reasonable, since it's a new pattern.

Thank you for the workaround regardless!