feat!(previewer): replace plenary.filetype with vim.filetype.match

[Conni2461]

CC @clason

this would drop usage of plenary.filetype

still left todo

• remove old documentation

seems like vim.filetype match doesnt have support for modeline so we still do that manually
:lua print(vim.filetype.match({ contents = { " vim:tw=78:ts=8:ft=help:norl:" } }))

[clason]

That's right, modeline is not part of the automatic filetype detection; this is an independent codepath (for manually setting the filetype independent of automatic detection).

[Conni2461]

okay, i think everything still works. doing docs a little bit later.

@fdschmidt93 can you review the changes in buffer_previewer.lua? because you've written parts of it. thanks :)

i probably gonna run it a couple of days before merging.

problem right now is that we cant really drop filetype support from plenary. https://sourcegraph.com/search?q=context%3Aglobal+plenary.filetype&patternType=standard&sm=1&groupBy=repo but we can work towards that, deprecating it, making a release, ....

[fdschmidt93]

Happy to have a look! As a heads up, planning to get to it Saturday morning.

[fdschmidt93]

I ran the {filesize, timeout}_hooks and filesize_limit and everything worked as intended.

Diff also lgtm :)

[Conni2461]

thanks for reviewing, then i'll try to get this done (and merged) by this evening

[clason]

(On a related note: It's a big job, but it would be worth making a dedicated effort to remove the dependency on plenary as much as possible. The Neovim standard library is much more extensive these days than it was when telescope was first written: there's vim.fs, vim.lpeg, vim.system (soon), vim.async (planned), ... May be worth a tracking issue or project board?)

[Conni2461]

yeah i thought about that as well, plenary is huge and there is a lot of stuff obsolete and its impossible to remove stuff from it. Like there is still async_v1 code in that repo and so on.

We probably will keep plenary for busted but yeah that would be the long term goal (to get rid of plenary).

Although i am not sure how we can get rid of plenary.log, plenary.popup, plenary.strings (we had that one here and upstreamed it to plenary) and we still have a couple of plenary.job

So yeah maybe the goal is to go to tag plenary and then start removing stuff. I'll make an "tracking issue" (here in telescope)

[Conni2461]

Marked as a breaking change because it could affects ppls config, in this case plenary.filetype. But i haven't seen any regressions (i know i said that before) so here goes nothing.

[clason]

I just noticed a regression: previewing .tex LaTeX files (without treesitter parser installed) is treated as "Binary cannot be previewed". Not all, though; some are just shown without highlighting (filetype) at all.

(I'm using VimTeX, which registers as a ftplugin for these, and includes its own ftdetect -- maybe that interferes? .sty files are recognized correctly, though.)

[clason]

Similar for cpp header files (e.g., src/klib/kbtree.h in the Neovim repo): no filetype, no highlighting.

Both of these require looking at contents to disambiguate. Maybe that's the issue here?

[Conni2461]

yep can reproduce, :| lets see if i can fix it

[Conni2461]

tex files arent being recognized even with full buffer as content input, thats a problem ...
same for .h files

[clason]

Is there a feature missing in vim.filetype?

[Conni2461]

currently skimming source code, one thing to notice is that there is not tex extension, only latex, using .latex on my file works.

header are a little bit difficult because its defined by a function, and those function, as far as i can see, always need to content already in the buffer to determine the ft, so passing content wouldn't resolve the issue, we could do the ft detect after we've written the lines to a buffer, but that would be a hack because we already read the lines

[clason]

currently skimming source code, one thing to notice is that there is not tex extension

If that were the case, Neovim couldn't detect .tex files at all, which is definitely the case.

header are a little bit difficult because its defined by a function, and those function, as far as i can see, always need to content already in the buffer to determine the ft, so passing content wouldn't resolve the issue, we could do the ft detect after we've written the lines to a buffer, but that would be a hack because we already read the lines

Yeah, I can see how that's an issue. But not detecting filetypes at all is not acceptable, I feel. Maybe do a first pass with only the file name, and if that fails a second pass with the read lines, and if that fails a third pass (which should rarely be needed) with the full file contents?

[Conni2461]

If that were the case, Neovim couldn't detect .tex files at all, which is definitely the case.

at least its missing in runtime/lua/vim/filetype.lua

[clason]

you need to also check detect.lua, which is where the content-based detection lives

[Conni2461]

Yeah, I can see how that's an issue. But not detecting filetypes at all is not acceptable, I feel. Maybe do a first pass with only the file name, and if that fails a second pass with the read lines, and if that fails a third pass (which should rarely be needed) with the full file contents?

i can make it work if i pass the bufnr and give the bufnr a name, or if i pass additionally the filename

i can not make it work if i pass the content + filename. if i see that correctly the content is ever really used for shebangs and a couple of patterns patterns_text

you need to also check detect.lua, which is where the content-based detection lives

i know, i think i have a pretty good understanding now how it works. also my bad i found the tex entry now

[Conni2461]

I have a fix for it, but i wanna come back the the implementation ^^ meaning i think i can make the combination of filename + contents work

[clason]

i can not make it work if i pass the content + filename.

That may be one of the (very few) combinations that is indeed missing, and probably should be added (in Neovim) if so.

if i see that correctly the content is ever really used for shebangs and a couple of patterns patterns_text

That is correct.