Closed mehalter closed 5 months ago
Thank you! Overall, these changes look great. However, I'm not entirely sure if the arguments of the set_from_buffer
function get handled properly. With your current implementation, calling :GuessIndent
with a buffer number (or any kind of argument for that matter), it always gets treated as if it were called by an auto_cmd, meaning that all user output get's supressed. Previously this was only the case when calling :GuessIndent auto_cmd
.
Do you think this the right behaviour? If a user goes out of their way to specify which buffer they want to be affected, I feel like they would probably also want to get feedback on what / if indentation got detected.
Thanks for this feedback! Yeah I thought about this as well. The main driving factor was to keep away from breaking changes to the API. The only way to avoid this would be to change the API to be better suited to being able to provide a buffer number. What do you think? Should I look at modifying the API in a way to better fit the ability to pass in the buffer number as well as specify the "auto_cmd" part?
@NMAC427 I just pushed a commit to resolve this, I separated out the bufnr
and autocmd
functionality for set_from_buffer
. This is a breaking change for that function definition. I updated :GuessIndent
to accept 2 arguments and provided details of that in comments in the code. The user facing command is not a breaking change with the extra parsing to make it hopefully not noticeable to the user. I looked at the docs and they don't seem to promote directly using the exposed Lua API so hopefully it won't affect too many users that the function signature for set_from_buffer
has changed!
Let me know what you think or if you have any other ideas for this improvement. Thanks again for considering this!
Ok, I have been noodling about this quite a bit and I think I have come up with a good interface for the :GuessIndent
command that satisfies all of the needs. The user can supply several arguments to control the indentation detection. They can specify: a specific buffer number, "context" (or "auto_cmd" for legacy support) to respect the buffer's context, and "silent" to silence the notification. Here are some examples to clarify:
:GuessIndent -- current buffer, no context, show notification
:GuessIndent context -- current buffer, respect context, show notification
:GuessIndent auto_cmd -- same as above, left in for legacy support
:GuessIndent context silent -- current buffer, respect context, no notification
:GuessIndent silent -- current buffer, no context, no notification
:GuessIndent 10 -- buffer 10, no context, show notification
:GuessIndent 10 context silent -- buffer 10, respect context, no notification
Based on the implementation, the arguments can be supplied in any order and only the first number is used as a buffer number.
Hopefully this deep dive and testing out different APIs is found to be useful! I really love this plugin and the approach it takes on detecting indentation and love the opportunity to help with the development 🤗
I have been running into a bug where after I open a file, any other file opened with :e
throws an error (yet still opens):
This branch fixed my problem, please take a look @NMAC427 :)
@NMAC427 Hey! Hope all is well, just wanted to check in and see if you have gotten any time to take a look at this PR now that it's been cleaned up. Thanks so much!
Sorry for not getting back to you sooner. Thank you for the great work!
No worries at all! Thanks so much for the consideration!
Hey! This is such a great plugin and I wanted to give a bit of a code refresh while also improving the API slightly by allowing specific buffers to be provided for the indentation detection. So there are a few goals of this PR:
Being able to provide specific buffer numbers as context. The functions operate on the "current buffer" which works ~99% of the time but every once in a while a buffer may be opened in the background of the current buffer which can cause the detection to not happen. This adds the ability to provide a number as the context to set to a specific buffer even if it isn't the current. I also improved the auto commands to use this new feature and pass the context from the autocommand itself which resolves these cases where buffers are opened in the background!
Use the more modern Lua APIs for the code. This removes all of the vimscript strings that were in the original codebase (for setting the user commands and autocmds). These have been around for a while and it is safe to adopt these. This makes things slightly faster (but we are juggling milliseconds). This also moves away from a couple APIs that are being deprecated in upcoming Neovim releases. Specifically
vim.api.nvim_buf_set_option
and replaces it with the simpler table notationUtilize the Lua Language Server annotations to document the API and make it easier to maintain down the line. This improves the type safety a bit and also provides LSP integration to the user if they use it. Now if the user does
require("guess-indent").setup({ })
they will get auto completion within the table passed to thesetup
function!Definitely the main goal of the PR was to add the buffer number specification and improve the autocommands/user commands, but thought that maybe a bit of a refactor could help future-proof the plugin a bit as Neovim continues to grow and evolve without increasing burden on the plugin maintainer. Let me know if you have any comments or concerns! Thanks again for this great plugin, it continues to work perfectly!