vim.filetype.add: user extensions can't override builtin patterns

[levioneyh]

Problem

Related: #19996

.s and .S files are not forced and instead recognized as the default "asm" filetype. I've tried:

1. Put the code in .config/nvim/after/ftdetect/ and .config/nvim/ftdetect directories.
2. Delete .local/share/nvim and .local/state/nvim.
3. Repeat the same steps for an arch VM without GUI where i never downloaded any nvim plugin or made any configuration changes.
4. Download the nvim.appimage (NVIM v0.11.0-dev-302+gbe999e6a0) from releases and repeat steps 1 and 2.

Steps to reproduce

In init.lua:

vim.filetype.add({
	extension = {
		c = "lua",
		TEST = "lua",
		s = "lua",
		S = "lua",
		asm = "lua",
	},
})

only .c, .TEST, and .asm files are overriden correctly

Expected behavior

.s and .S files are correctly forced to the specified types.

Neovim version (nvim -v)

NVIM v0.10.0 Build type: Release LuaJIT 2.1.1716656478

Vim (not Nvim) behaves the same?

Operating system/version

Arch linux, 6.9.6-arch1-1

Terminal name/version

kitty, linux console

$TERM environment variable

xterm-kitty, linux

Installation

Pacman

[zeertzjq]

It works if you add it in the pattern table:

vim.filetype.add({
	extension = {
		c = "lua",
		TEST = "lua",
		asm = "lua",
	},
	pattern = {
		['.*%.[sS]'] = "lua",
	},
})

This is because patterns always have priority over extensions. However, maybe there should be a way for user extensions to override builtin patterns?

[clason]

However, I think there should be a way for user extensions to override builtin patterns.

That is difficult due to the architecture of the filetype detection (if not impossible without a full, burn-it-to-the-ground-first, redesign).

I think this should just be documented better, including with a concrete example. (The vim.filetype.add documentation implies this, but only after you have grokked the logic.)

[zeertzjq]

Moving asm patterns to the extension table in Nvim will also make this work:

diff --git a/runtime/lua/vim/filetype.lua b/runtime/lua/vim/filetype.lua
index 6dd920e506..48685368f8 100644
--- a/runtime/lua/vim/filetype.lua
+++ b/runtime/lua/vim/filetype.lua
@@ -205,6 +205,10 @@ local extension = {
   asm = detect.asm,
   lst = detect.asm,
   mac = detect.asm,
+  s = detect.asm,
+  S = detect.asm,
+  a = detect.asm,
+  A = detect.asm,
   asn1 = 'asn',
   asn = 'asn',
   asp = detect.asp,
@@ -1741,8 +1745,6 @@ local pattern = {
   ['.*asterisk/.*%.conf.*'] = starsetf('asterisk'),
   ['.*asterisk.*/.*voicemail%.conf.*'] = starsetf('asteriskvm'),
   ['.*/%.aptitude/config'] = 'aptconf',
-  ['.*%.[aA]'] = detect.asm,
-  ['.*%.[sS]'] = detect.asm,
   ['[mM]akefile%.am'] = 'automake',
   ['.*/bind/db%..*'] = starsetf('bindzone'),
   ['.*/named/db%..*'] = starsetf('bindzone'),

I'm not sure if this should be done, as it's indeed done for some filetypes, but for some other filetypes (e.g. case-insensitive one) this may not be a good idea.

[clason]

It's also a matter of performance. I don't think we should mess with the logic; it was hard-won. Instead, we just need to explicitly mention that you can only override within a category, so pick the highest one (filename > pattern > extension) that fits.

[zeertzjq]

I feel that the current behavior isn't ideal. The pattern vs. extension for builtin filetypes is an implementation detail, but the behavior of vim.filetype.add() is kind of affected by that.

[clason]

It's not ideal, but the efficiency and robustness of the implementation (which is build around the category cascade) is more important IMO -- especially since you can override, you just have to know how.

@gpanders @smjonas

[levioneyh]

Thanks for the solution @zeertzjq, FWIW I find it unintuitive that some extensions can be overridden, while others can't due to some internal nvim code, even now knowing this priority logic.

[clason]

You can override extensions by extensions, but not patterns by extensions. I agree it's unintuitive since it requires knowing implementation details, but there are really good reasons for the way things are now. If in doubt, specify a pattern.

[gpanders]

Moving asm patterns to the extension table in Nvim will also make this work:

diff --git a/runtime/lua/vim/filetype.lua b/runtime/lua/vim/filetype.lua
index 6dd920e506..48685368f8 100644
--- a/runtime/lua/vim/filetype.lua
+++ b/runtime/lua/vim/filetype.lua
@@ -205,6 +205,10 @@ local extension = {
   asm = detect.asm,
   lst = detect.asm,
   mac = detect.asm,
+  s = detect.asm,
+  S = detect.asm,
+  a = detect.asm,
+  A = detect.asm,
   asn1 = 'asn',
   asn = 'asn',
   asp = detect.asp,
@@ -1741,8 +1745,6 @@ local pattern = {
   ['.*asterisk/.*%.conf.*'] = starsetf('asterisk'),
   ['.*asterisk.*/.*voicemail%.conf.*'] = starsetf('asteriskvm'),
   ['.*/%.aptitude/config'] = 'aptconf',
-  ['.*%.[aA]'] = detect.asm,
-  ['.*%.[sS]'] = detect.asm,
   ['[mM]akefile%.am'] = 'automake',
   ['.*/bind/db%..*'] = starsetf('bindzone'),
   ['.*/named/db%..*'] = starsetf('bindzone'),

I'm not sure if this should be done, as it's indeed done for some filetypes, but for some other filetypes (e.g. case-insensitive one) this may not be a good idea.

I think regardless of whatever other decisions we make, we should do this. There's no reason for the pattern table to have patterns like this, this is exactly what the extension table is for. If there are other examples like this, we should move those too.