Skip to content

Commit

Permalink
Rework epp subtype detection
Browse files Browse the repository at this point in the history
The previous work had some problems:

* it was working for nvim but not at all for vim.
* it did not provide any loop-breaking checks before launching doautocmd
* it was setting `filetype` from within the syntax file instead of the
  ftdetect file.

This reworks the code to fix most of the above issues. The changes are
almost verbatim what Time Pope wrote for the eruby syntax and ftplugin
files in vim/nvim.

With the changes, vim now is able to correctly identify the subtype
based on the file name's extension prior to .epp. This covers most of the
basic cases.

However, I was not able to find a good way to solve finding the filetype
based on parsing file contents and the paths leading to the file. We
don't want to reimplement the whole of builtin file detection, which is
why @shadowwa was trying to reuse the autocommands already determined.

Until we find a way to fix the above issue, we at least now have most of
the filetype detection working, which is better than nothing.

NOTE: I'm intentionally keeping the tests that are failing for vim in
this commit. This way ppl who will investigate this project in the
future will be able to see that the tests actually pass on nvim, but not
on vim.
The next commit will comment out the failing tests though, since
vader.vim still doesn't have a SkipIf instruction.
See: junegunn/vader.vim#201
  • Loading branch information
lelutin committed Aug 28, 2024
1 parent e5ac7cb commit 4938353
Show file tree
Hide file tree
Showing 5 changed files with 108 additions and 43 deletions.
2 changes: 1 addition & 1 deletion ftdetect/puppet.lua
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ vim.filetype.add({
path_wo_epp = path:sub(1,-5)
matched = vim.filetype.match({ buf = bufnr, filename = path_wo_epp })
if matched ~= nil and matched ~= 'mason' then
vim.b.original_filetype = matched
vim.b.epuppet_subtype = matched
end
return 'epuppet'
end,
Expand Down
43 changes: 21 additions & 22 deletions ftdetect/puppet.vim
Original file line number Diff line number Diff line change
@@ -1,24 +1,23 @@
" vint: -ProhibitAutocmdWithNoGroup
" Vim's own filetypes.vim runs before all ftdetect scripts (why?) and matches
" detects the .pp extension as being a 'pascal' file. Since the script uses
" `setf`, we can nullify the filetype detection by removing all commands bound
" to BufRead and BufNewFile for .pp files with `au!`. Hopefully, if there were
" any other commands set they were associated with the pascal type and we want
" to get rid of them.
" However, this has the effect of completely nullifying pascal type detection
" for .pp files.
au! BufRead,BufNewFile *.pp setfiletype puppet
" Some epp files may get marked as "mason" type before this script is reached.
" Vim's own scripts.vim forces the type if it detects a `<%` at the start of
" the file. All files ending in .epp should be epuppet
if !has('nvim-0.8.0')
autocmd! BufRead,BufNewFile *.epp call DetectOriginalType()
function! DetectOriginalType()
execute 'doautocmd filetypedetect BufRead ' .fnameescape(expand('<afile>:r'))
if &filetype !=# '' && !( &filetype ==# 'mason' && expand('<afile>') !~# 'mason')
let b:original_filetype = &filetype
endif
setlocal filetype=epuppet
endfunction
" Vim has fixed puppet vs pascal detection in patch 8.2.2334 so we can rely on
" their type detection from that point on.
if !has("patch-8.2.2334") && !has("nvim-0.5.0")
" Vim's own filetypes.vim runs before all ftdetect scripts (why?) and matches
" detects the .pp extension as being a 'pascal' file. Since the script uses
" `setf`, we can nullify the filetype detection by removing all commands bound
" to BufRead and BufNewFile for .pp files with `au!`. Hopefully, if there were
" any other commands set they were associated with the pascal type and we want
" to get rid of them.
" However, this has the effect of completely nullifying pascal type detection
" for .pp files.
au! BufRead,BufNewFile *.pp setfiletype puppet
endif
" Vim now has autodetection for epuppet and Puppetfile. We only need to add
" autocommands for older versions of vim / neovim
if !has("patch-8.2.2402") && !has("nvim-0.5.0")
" Some epp files may get marked as "mason" type before this script is reached.
" Vim's own scripts.vim forces the type if it detects a `<%` at the start of
" the file. All files ending in .epp should be epuppet
au! BufRead,BufNewFile *.epp setf epuppet
au BufRead,BufNewFile Puppetfile setfiletype ruby
endif
au BufRead,BufNewFile Puppetfile setfiletype ruby
41 changes: 40 additions & 1 deletion ftplugin/epuppet.vim
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,48 @@ set cpo-=C

" Define some defaults in case the included ftplugins don't set them.
let s:undo_ftplugin = ''
let s:browsefilter = "All Files (*.*)\t*.*\n"
if has("win32")
let s:browsefilter = "All Files (*.*)\t*.*\n"
else
let s:browsefilter = "All Files (*)\t*\n"
endif
let s:match_words = ''

if !exists("g:epuppet_default_subtype")
let g:epuppet_default_subtype = "sh"
endif

if &filetype =~ '^epuppet\.'
let b:epuppet_subtype = matchstr(&filetype,'^epuppet\.\zs\w\+')
elseif !exists('b:epuppet_subtype')
let b:epuppet_subtype = matchstr(substitute(expand("%:t"),'\c\%(\.epp\)\+$','',''),'\.\zs\w\+\%(\ze+\w\+\)\=$')
" TODO instead of listing exceptions like this, can we instead recognize
" extension -> type mapping?
if b:epuppet_subtype == 'rhtml'
let b:epuppet_subtype = 'html'
elseif b:epuppet_subtype == 'rb'
let b:epuppet_subtype = 'ruby'
elseif b:epuppet_subtype == 'yml'
let b:epuppet_subtype = 'yaml'
elseif b:epuppet_subtype == 'js'
let b:epuppet_subtype = 'javascript'
elseif b:epuppet_subtype == 'txt'
" Conventional; not a real file type
let b:epuppet_subtype = 'text'
elseif b:epuppet_subtype == 'py'
let b:epuppet_subtype = 'python'
elseif b:epuppet_subtype == 'rs'
let b:epuppet_subtype = 'rust'
elseif b:epuppet_subtype == ''
let b:epuppet_subtype = g:epuppet_default_subtype
endif
endif

if exists("b:epuppet_subtype") && b:epuppet_subtype != '' && b:epuppet_subtype !=? 'epuppet'
exe "runtime! ftplugin/".b:epuppet_subtype.".vim ftplugin/".b:epuppet_subtype."_*.vim ftplugin/".b:epuppet_subtype."/*.vim"
endif
unlet! b:did_ftplugin

runtime! ftplugin/sh.vim
unlet! b:did_ftplugin

Expand Down
47 changes: 34 additions & 13 deletions syntax/epuppet.vim
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,40 @@ if exists('b:current_syntax')
finish
endif

if exists('b:original_filetype')
runtime! syntax/'.b:original_filetype .'.vim'
" allow original filetype detection by other plugins like ale or coc.nvim
let &filetype=b:original_filetype .'.epuppet'
else
runtime! syntax/sh.vim
if !exists("g:epuppet_default_subtype")
let g:epuppet_default_subtype = "sh"
endif

if &filetype =~ '^epuppet\.'
let b:epuppet_subtype = matchstr(&filetype,'^epuppet\.\zs\w\+')
elseif !exists('b:epuppet_subtype')
let b:epuppet_subtype = matchstr(substitute(expand("%:t"),'\c\%(\.epp\)\+$','',''),'\.\zs\w\+\%(\ze+\w\+\)\=$')
" TODO instead of listing exceptions like this, can we instead recognize
" extension -> type mapping?
if b:epuppet_subtype == 'rhtml'
let b:epuppet_subtype = 'html'
elseif b:epuppet_subtype == 'rb'
let b:epuppet_subtype = 'ruby'
elseif b:epuppet_subtype == 'yml'
let b:epuppet_subtype = 'yaml'
elseif b:epuppet_subtype == 'js'
let b:epuppet_subtype = 'javascript'
elseif b:epuppet_subtype == 'txt'
" Conventional; not a real file type
let b:epuppet_subtype = 'text'
elseif b:epuppet_subtype == 'py'
let b:epuppet_subtype = 'python'
elseif b:epuppet_subtype == 'rs'
let b:epuppet_subtype = 'rust'
elseif b:epuppet_subtype == ''
let b:epuppet_subtype = g:epuppet_default_subtype
endif
endif

if exists("b:epuppet_subtype") && b:epuppet_subtype != '' && b:epuppet_subtype !=? 'epuppet'
exe "runtime! syntax/".b:epuppet_subtype.".vim"
unlet! b:current_syntax
endif
unlet! b:current_syntax

syn include @puppetTop syntax/puppet.vim

Expand All @@ -31,9 +57,4 @@ syn region ePuppetComment matchgroup=ePuppetDelimiter start="<%-\=#" end=
hi def link ePuppetDelimiter PreProc
hi def link ePuppetComment Comment

if exists('b:original_filetype')
let b:current_syntax = b:original_filetype . '.epuppet'
else
let b:current_syntax = 'epuppet'
endif

let b:current_syntax = 'epuppet'
18 changes: 12 additions & 6 deletions test/filetype/epuppet.vader
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,26 @@ Execute (epuppet test_with_leading_tag):

Execute (epuppet perl with shebang):
edit test/test-files/test_perl_with_shebang.epp
AssertEqual &filetype, 'perl.epuppet'
AssertEqual &filetype, 'epuppet'
AssertEqual b:epuppet_subtype, 'perl'

Execute (epuppet shell with shebang):
# We don't need to parse the shebang for shell since sh is the default subtype
Execute (epuppet default to shell):
edit test/test-files/test_shell_with_shebang.epp
AssertEqual &filetype, 'sh.epuppet'
AssertEqual &filetype, 'epuppet'
AssertEqual b:epuppet_subtype, 'sh'

Execute (epuppet shell with extension):
edit test/test-files/test_shell_with_extension.sh.epp
AssertEqual &filetype, 'sh.epuppet'
AssertEqual &filetype, 'epuppet'
AssertEqual b:epuppet_subtype, 'sh'

Execute (epuppet php with extension):
edit test/test-files/test_php_with_extension.php.epp
AssertEqual &filetype, 'php.epuppet'
AssertEqual &filetype, 'epuppet'
AssertEqual b:epuppet_subtype, 'php'

Execute (epuppet apache conf with path and extension):
edit test/test-files/etc/apache2/test.conf.epp
AssertEqual &filetype, 'apache.epuppet'
AssertEqual &filetype, 'epuppet'
AssertEqual b:epuppet_subtype, 'apache'

0 comments on commit 4938353

Please sign in to comment.