andrewferrier / debugprint.nvim

Debugging in NeoVim the print() way!
MIT License
313 stars 17 forks source link

Support lazy loading #97

Closed aloknigam247 closed 5 months ago

aloknigam247 commented 5 months ago

The new change is breaking as now we cannot lazy load with mappings.

For lazy plugin manager, we could have lazy loading as below

{
    'andrewferrier/debugprint.nvim',
    opts = {
        keys = {
            { '<Leader>dP', function() return require('debugprint').debugprint({ above = true }) end,                  expr = true, mode = 'n' },
            { '<Leader>dV', function() return require('debugprint').debugprint({ above = true, variable = true }) end, expr = true, mode = 'n' },
            { '<Leader>dV', function() return require('debugprint').debugprint({ above = true, variable = true }) end, expr = true, mode = 'v' },
            { '<Leader>dd', function() return require('debugprint').deleteprints() end,                                             mode = 'n' },
            { '<Leader>dp', function() return require('debugprint').debugprint() end,                                  expr = true, mode = 'n' },
            { '<Leader>dv', function() return require('debugprint').debugprint({ variable = true }) end,               expr = true, mode = 'n' },
            { '<Leader>dv', function() return require('debugprint').debugprint({ variable = true }) end,               expr = true, mode = 'v' },
        }
    }
}

Now it is not possible as all the mappings and commands are created with auto command for event {"BufReadPost", "BufNewFile"}. When keys are executed for loading the plugin these auto commands are already processed for current buffer and works with new buffers onwards.

I have 2 solutions in mind -

  1. Instead of creating mappings for each buffer we can create it globally once in setup. Lazy loading will work in this case.
  2. Extract mapping callback into different function and execute it once in setup and again with auto command.

I can raise a PR if needed.

andrewferrier commented 5 months ago

@aloknigam247 I understand. It was a difficult choice to make this change but it was due to repeated confusion on how to perform the keymapping, especially the use of expr = true (see https://github.com/andrewferrier/debugprint.nvim/issues/44#issuecomment-1896405231 for the history).

Solution (1) would effectively revert this change which ensures that the keybindings are not set in non-modifiable buffers, which is also desirable. However, I have been thinking that that restriction is not ideal in any case, and I should probably replace it with a simple warning message that the buffer is not modifiable - this is what NeoVim itself does, for example, if you try to enter insert mode on a non-modifiable buffer.

Any comments welcome, but I shall probably make that change shortly.

aloknigam247 commented 5 months ago

@andrewferrier , I read the comment, and I understood that the motivation behind this change was to keep the mapping options as expected the plugin author and keep the keys configurable only. But I wonder how auto command is helpful here, because it will execute for all buffers, at the end behaving as if global mappings are set.

And about the concern of giving a warning in non-modifiable buffers, vim already gives a warning when we try to execute these mapping. So, if you are going to create a check before execution of the mapping, wouldn't this be redundant and increase mapping execution time?

I may not have understood the concern completely, please correct me where am I wrong.

andrewferrier commented 5 months ago

@aloknigam247 I agree the autocmd is clumsy. I've just pushed a change which removes that, and instead just issues a warning when using debugprint in non-modifiable buffers. I think that's cleaner and clearer.

I also think that this should work better with lazy-loading. Note though that as of v2.0.0 you shouldn't map to require('debugprint').debugprint directly in your lazy configuration like you're showing; see the README for explanation.

Note that this change is only on the main branch. If you can confirm it works, I'll promote it into a release.

andrewferrier commented 5 months ago

@aloknigam247 thanks. I've just pushed a change which should fix this by removing the autocmd and warning on use in nonmodifiable buffers instead. Please retest the latest version on main; if that works I'll release it as a numbered release.

Note that you shouldn't map to keys directly using debugprint.debugprint() any more; see this section of the README for information. I've also extended the sample setup for lazy.nvim.

aloknigam247 commented 5 months ago

Thanks @andrewferrier. I am lazy loading as mentioned in wiki also, and it is working as expected. Appreciate quick response from you. Thank you for maintaining such a good plugin.

andrewferrier commented 5 months ago

@aloknigam247 you're welcome! Thanks for confirming.