runtime(ftplugin): make ft-typescript use javascript.vim #17168
Conversation
Typescript (and JavascriptReact, TypescriptReact) are just super-sets of Javascript. So to avoid duplication, we should make them source ft-javascript.
ecc8ef2 to
22a76c9
Compare
| @@ -0,0 +1,14 @@ | |||
| " Change the :browse e filter to primarily show JavaScript-related files. | |||
| if (has("gui_win32") || has("gui_gtk")) && !exists("b:browsefilter") | |||
There was a problem hiding this comment.
This snippet is moved from javascript.vim to here.
I am not sure if it is a good idea to filter Javascript-like files only. Because in web development, we could also use HTML, CSS, EJS beside Javascript.
Not to say that this behavior is not documented (which means it is unexpected to users), so should this be removed?
There was a problem hiding this comment.
I am not sure if it is a good idea to filter Javascript-like files only. Because in web development, we could also use HTML, CSS, EJS beside Javascript.
I agree it's useful to include related files, and have a local copy adding HTML and CSS, but it was never clear where to stop with this. It seems reasonable to limit the filters to the actual filetype and utilise the "All Files" filter for anything else.
Are you suggesting the order should be changed to default to "All files"? That's the default behaviour but the filetype plugins have always put that last, I think.
Lines 2509 to 2525 in 31b78cc
Not to say that this behavior is not documented (which means it is unexpected to users), so should this be removed?
I'm not sure what needs to be documented as the filtering is discoverable simply by opening the file dialog. Rather than remove it, I think a browsefilter value should be set for all filetypes.
There was a problem hiding this comment.
I don't have a strong opinion on whether to keep it or remove it or add what, but I think this kind of behavior should be documented and consistent between different filetypes.
I'm not sure what needs to be documented as the filtering is discoverable simply by opening the file dialog.
I would never know what kind of filetypes it filters without trying it several times. That is bad UX.
There was a problem hiding this comment.
You mean the (has("gui_win32") || has("gui_gtk")) ? Or what exactly should be documented? The browsefilter variable is defined for "Windows, GTK and the Motif UI". But although Motif GUI supports the browsefilter variable, there is no easy way to switch different filters. So restricting it to gui_win32 or gui_gtk makes sense.
| setlocal suffixesadd+=.ts,.d.ts,.tsx,.js,.jsx,.cjs,.mjs | ||
|
|
||
| let b:undo_ftplugin = "setl fo< com< cms< sua<" | ||
| runtime! ftplugin/javascript.vim |
There was a problem hiding this comment.
Does it make sense to set the omnifunc from Javascript also for Typescript? Similar for the 'define' option and matchit values?
There was a problem hiding this comment.
For what it's worth, I came up with these settings for typescript, but @romainl surely has better founded ideas on their right values
setlocal include=^\\s*\\%(import\\|export\\).*\\<from\\>\\s*['\"]\\zs[^'\"\\r\\n]\\+\\ze['\"]\\|\\<require(\\s*['\"]\\zs[^'\"\\r\\n]\\+\\ze['\"]
let &l:define = '^\s*\('
\..'\(export\s\)*\(default\s\)*\(var\|const\|let\|function\|class\|interface\)\s'
\..'\|\(public\|private\|protected\|readonly\|static\)\s'
\..'\|\(get\s\|set\s\)'
\..'\|\(export\sdefault\s\|abstract\sclass\s\)'
\..'\|\(async\s\)'
\..'\|\(\ze\i\+([^)]*).*{$\)'
\..'\)'
if exists('b:match_words')
let b:match_words .= ','
else
let b:match_words = ''
endif
let b:match_words = '\<function\>:\<return\>,'
\..'\<do\>:\<while\>,'
\..'\<switch\>:\<case\>:\<default\>,'
\..'\<if\>:\<else\>,'
\..'\<try\>:\<catch\>:\<finally\>'There was a problem hiding this comment.
Just stumbled onto https://github.com/HerringtonDarkholme/yats.vim/blob/master/ftplugin/typescript.vim#L91 ; an include expression is also useful
There was a problem hiding this comment.
I also have an includeexpr that can even jump to Node modules https://github.com/brianhuster/dotfiles/blob/main/.config/nvim/after/ftplugin/javascript.vim
But I don't think it will be very useful because of #16707, and also language server covers this feature well.
There was a problem hiding this comment.
language server covers this feature well.
Ctags and Language Servers take you further, but we're here to max out Vim's built-in functionalities. This reminds me of zzzyxwvut/java-vim#4 (comment) ; even if &include sometimes breaks on gf, its principal use is including other files for grepping #16916 (comment)
There was a problem hiding this comment.
but we're here to max out Vim's built-in functionalities
Just think how many people use the built-in omnifunc of JavaScript in 2025?
I will not add it to this PR, but if you want to do it for your own PR, you are free to do that.
There was a problem hiding this comment.
noone even did anything, until I did.
That's commendable, keep up the good work
I will not add it to this PR, but if you want to do it for your own PR, you are free to do that.
Okay, one step after another.
There was a problem hiding this comment.
Btw, a formatexpr for Vimscript and help will be nice, I think these Vim's specific languages need "maximized Vim functionalities" the most.
There was a problem hiding this comment.
Yes, makes sense, a separate issue. For help.vim, there's HelpFormatExpr floating around
|
Since python and other plug-ins sport these as well, how about mappings to move between functions |
I see many mappings, I think we should only choose 2 operations and map to But anyway, it's not purpose of this PR, so I think that should be discussed in another issue |
Yes, in principle. What confuses me a bit in general is the distinction between navigating between methods |
|
are you still working on that @brianhuster ? |
|
I don't abandon this, but it's true that this is not very high in my priority list (since this is just a refactor, not a fix). So anyone can take over this |
|
thanks and no worries. Take your time, it may just push down in my attention list :) |
|
Btw, based on the test, it seems that |
22a76c9 to
492d0de
Compare
|
Could I put the file |
Typescript (and JavascriptReact, TypescriptReact) are just super-sets of Javascript. So to avoid duplication, I think we should make them source ft-javascript.
I would like some reviews from @dkearns @chrisbra