fix: explicitly set the sql filetype on new notes
#207
Conversation
| buffer_options = vim.tbl_extend("force", { | ||
| buflisted = false, | ||
| swapfile = false, | ||
| filetype = "sql", | ||
| }, opts.buffer_options or {}), |
There was a problem hiding this comment.
This shouldn't affect users who set their own filetype in their configs, because the user configs should override the default config.
There was a problem hiding this comment.
I think this makes sense as the default setting here. There are some other adapters that are not SQL file types, but that can as you said be handled by the user overwriting.
Maybe as a future improvement, the editor buffer could be handler aware, to set the proper one directly. But I think that would require some refactoring to enable.
Thoughts @MattiasMTS ?
There was a problem hiding this comment.
I think this makes sense as the default setting here. There are some other adapters that are not SQL file types, but that can as you said be handled by the user overwriting.
Maybe as a future improvement, the editor buffer could be handler aware, to set the proper one directly. But I think that would require some refactoring to enable.
Thoughts @MattiasMTS ?
There was a problem hiding this comment.
2cents. I'm ok merging this given, as raised in the PR, this plugin is mainly SQL focused but we also have noSQL drivers. However, I expect that noSQL users already have some overwritten logic themselves already, so this PR doesn't break that.
Although, final words, I'd want to see this as part of the source in relation with the connection struct. For example, in each source/driver one would declare what filetype should be set - and then that is set dynamically as such. This is more future PR refactoring and no within the scope of this PR
|
Thanks for the feedback @phdah @MattiasMTS! What else do I need to do, to get this merged? |
It's @MattiasMTS that has the power to merge. I think he can just go ahead an do it? |
|
Sorry late reply - lots of thing at work. Yes I can merge, but the CI/CD will fail. Could we potentially merge #204 first with the proposed changes and then merge this. Sounds ok? |
This fixes #154.
For some reason, when creating a new note (local or global), the filetype for the new buffer was being set to
sql, but treesitter wasn't picking up the filetype for syntax highlighting.This is a bit of a hack, but I found that explicitly setting the filetype to
sqlon the buffer seems to resolve the issue 🤷