[Enhancement] Try lsp_rename but default to current_word

[tummetott]

Hello,

First and foremost, I'd like to express my appreciation for this excellent plugin.

While reading through the readme documentation, I noticed that there are quite a few suggested keymaps for usage . I believe that when someone is in the process of refactoring a variable to a different case, and a language server is active, the natural inclination is to use the rename feature provided by the language server. Only in cases where there is no language server running would a simple refactor of the current word be desired. Is it truly necessary to have separate keymaps for these scenarios?

I've considered the possibility of combining lsp_rename and current_word into a single keymap function, but this approach seems to introduce a significant amount of boilerplate code, as shown here:

local toSnakeCase = function()
    local clients = vim.lsp.get_active_clients()
    for _, client in ipairs(clients) do
        if client.server_capabilities.renameProvider then
            require('textcase').lsp_rename('to_snake_case')
            return
        end
    end
    require('textcase').current_word('to_snake_case')
end

vim.keymap.set('n', '<leader>cs', toSnakeCase, { desc = 'to snake case' })

This led me to wonder if it might be feasible to integrate this behavior directly into the plugin. By doing so, you could expose just one function to users, simplifying the keymapping process and enhancing the overall user experience.

[johmsalas]

Appreciate your kind words.

I've been considering the different edge cases of this implementation. Specifically, on adding this behavior to one of the existing methods. LSP rename is a better fit to include a gracefully degraded behavior that changes the current word when there are no LSP attached servers.

What stops me on this approach is the user expects a LSP renaming. If there are no available servers, in fact, the word casing is changed but the user does not get feedback it actually failed to LSP rename all the instances

Overall I agree on your suggestion. Do you have thoughts regarding the DX/UX?

[tummetott]

I totally get where you're coming from. The suggestion I made was more of a general concept. I wouldn't recommend altering any existing functions; perhaps adding a new function would be the way to go. This way, users can decide which one to map.

I have two other questions:

1. It appears that lsp_rename("to_camel_case") and to_pascal_case aren't functioning as expected, while to_snake_case works for me.

2. Can default key mappings be disabled? I prefer to customize my own keymaps and only keep the ones I need.

[johmsalas]

@tummetott it can be a new function as you suggested. Do you think you can create a PR? That one sounds easy since it is a new feature (non breaking change) and is based on existing behavior. If not I can add also add it

About your questions:

1. For the camel and pascal case functions, they seem to be working (according to the unit tests). Could be the LSP behavior. Is it attached to the buffer? Can you provide the reproduction steps? Going to suggest creating another issue (btw, it works on my machine :sigh)
2. I should add some unit tests to make sure it still works as initially designed, but it is supposed, you can omit keybindings by not calling the setup method.

[tummetott]

Hello @johmsalas,

After further experimentation with the suggested change, I've concluded that bundling lsp_rename and current_word doesn't make sense. Initially, I thought that when an LSP is attached, the server could rename any symbol under the cursor. However, this is not always true. For instance, with lua_ls, there are cases where the LSP reports "Language server couldn't provide rename result." In such scenarios, the individual mappings serve a more meaningful purpose. I apologize for any inconvenience caused by this matter.

By the way, not calling setup() successfully prevents the default keybindings from being loaded, and all other functions work as expected.

[johmsalas]

Something that resonated to me in the issue is there are too many keybindings, I didn't notice it until stopped programming for a while and then came back, there is a learning curve I'd like to get rid of.

How about attempt to lsp rename, if it fails.. for any reason, like not being available in the attached LSP, default to current_word?

[tummetott]

I had exactly the same thought but i had not idea how to probe if a LSP rename would work or fail without doing the actual rename. If you find a solution, that would be the best option i guess

[johmsalas]

This plugin uses buf_request, which seems to be deprecated o.O

It should be replace by buf_request_all({bufnr}, {method}, {params}, {handler}) that receives a callback (handler) with the result after attempting the rename

[johmsalas]

Related: #57

[johmsalas]

@tummetott I'm starting to work on #57 and will include an example of failed LSP rename

[johmsalas]

Added an example of LSP failure in this PR

In order to implement the LSP_rename_or_current_word method, we should create another method, similar to lsp_rename.
In the do_lsp_rename method, instead of showing an error, it would make the word replacement

[johmsalas]

I'm not sure if we want to call M.current_word or M.quick_replace 🤔
hadn't work on those for a while, will need to check the difference between both