kamranahmedse
[Suggestion] Blazor Roadmap
Not strongly opposed but the C defaults can be useful in plain text files and other files lacking better defaults. Could address that here by adding defaults for filetype=text at least.
What is cost of the current defaults?
Could address that here by adding defaults for filetype=text at least.
LaTeX defaults, clearly :D
What is cost of the current defaults?
There is not really a cost, but there is also not really any cost to removing the defaults and putting them in an ftplugin. The benefit is that it makes Nvim more coherent and consistent with the rule "filetype specific options go in filetype plugins".
the C defaults can be useful in plain text files and other files lacking better defaults
I am not sure what a good default value for these options would even be, since they are all language specific. I also cannot think of a case where the C default values would be useful outside of editing a C (or C++) file.
There is not really a cost,
There is a cost, we should be aware of it and decide based on that. E.g. after this change gf arguably is less useful for no real benefit. In fact we might want to augment the default 'path' rather than shrink it.
I also cannot think of a case where the C default values would be useful outside of editing a C (or C++) file.
Paths for example can show up in comments in various files, like markdown docs.
Paths for example can show up in comments in various files, like markdown docs.
Yes, but a full path still works with gf, as do relative paths. The only case in which having /usr/include in the default 'path' would be useful is if someone wanted to navigate to a C header file when the word under the cursor is just "stdio.h" (for example), but they are not already in a C source file.
And /usr/include is not even a valid path on macOS or Windows. So this is an extremely narrow use case.
I'm not going to push too hard for this if there's not a consensus that it's an easy choice. It's a low risk, low impact change, so not worth spending a lot of time on it.
+1 from me; for me this falls pretty squarely under the "sane defaults" goal of Neovim (emphasis on "sane").
But low impact either way.
I've just gotta say both include and define seem completely redundant and have little (or zero?) use outside of C/CPP, and even for those don't provide much value since they are unlikely to work for many projects.
path on the other hand seems quite generic and generally useful.
I think commentstring needs a non-empty default. Perhaps // %s.
default should be .,,, not empty, but yes C specific stuff like /usr/include can be removed and put in ftplugin
default should be
.,,,not empty
Indeed, and that is the value set in this PR.
Any chance we can pick this back up? I think we need this for ts-aware commentstrings. At the moment parsers like comment pick up the default commentstring value. We need it to be '' so we can ignore it.
I am generally in favor of these changes (in particular to commentstring, for the reason @lewis6991 mentions above); my only concern is that this involves "forking" the C ftplugin, which so far we've diligently avoided ("vim runtime belongs to Vim").
Would it be possible to set these in a separate ftplugin/c.lua instead to make clear that these are Neovim-specific (and allow easier merging of runtime file updates)?
Also, tests would have to be adapted, especially a bunch of oldtests...
The current PR removes defaults from
-
commentstring(this is a necessary change to make filetype options -- and thus treesitter injection options -- work) -
define(not useful outside of C/C++) -
include(not useful outside of C/C++) -
path(not useful outside of C/C++ on Unix)
All of these seem like clear improvements to me, but only the first is actually necessary. If this unblocks the PR for 0.9, I'm OK with separating out that change.
@justinmk Please review changes to vim_diff
@zeertzjq Please review changes to oldtests