Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(nui): allow callbacks in nui input with option #1372

Merged
merged 3 commits into from Mar 6, 2024

Conversation

pysan3
Copy link
Collaborator

@pysan3 pysan3 commented Mar 2, 2024

We already have an option to default to normal mode in nui inputs.
config.enable_normal_mode_for_inputs

However, it looks like users want more power of customization to the details. (#1371)

So why not just let users do whatever they want with absolute power?

All function/string/table are considered true-thy so this will cover backwards compatibility.

`config.enable_normal_mode_for_inputs`
@pysan3
Copy link
Collaborator Author

pysan3 commented Mar 2, 2024

Usecase:

Move cursor right before the extension and start insert mode.

config.enable_normal_mode_for_inputs = ":exe 'norm F.' | :startinsert"

@cseickel
Copy link
Contributor

cseickel commented Mar 2, 2024

Usecase:

Move cursor right before the extension and start insert mode.

config.enable_normal_mode_for_inputs = ":exe 'norm F.' | :startinsert"

😱 In this example you're using an option called enable_normal_mode_for_inputs to startinsert mode. 😱

I think we already have a generic solution that would be perfect, which is the event system. In a quick attempt I could not get the existing NEO_TREE_POPUP_BUFFER_ENTER event to work for this situation, but I think either modifying the timing of NEO_TREE_POPUP_BUFFER_ENTER or adding a new event which fires at the appropriate time is the correct way to create a generic solution. Maybe NEO_TREE_POPUP_INPUT_READY would be a good event name.

@pysan3
Copy link
Collaborator Author

pysan3 commented Mar 2, 2024

Usecase:
Move cursor right before the extension and start insert mode.

config.enable_normal_mode_for_inputs = ":exe 'norm F.' | :startinsert"

😱 In this example you're using an option called enable_normal_mode_for_inputs to startinsert mode. 😱

Yeah 🎉 That's what I mean by absolute power lol. I plan to not document this anywhere so it's absolutely to the users own risk to do anything.

I think we already have a generic solution that would be perfect, which is the event system. In a quick attempt I could not get the existing NEO_TREE_POPUP_BUFFER_ENTER event to work for this situation, but I think either modifying the timing of NEO_TREE_POPUP_BUFFER_ENTER or adding a new event which fires at the appropriate time is the correct way to create a generic solution. Maybe NEO_TREE_POPUP_INPUT_READY would be a good event name.

NEO_TREE_POPUP_BUFFER_ENTER This won't work as stopinsert is called way after buffer creation or even input:mount, and it's inside a vim.schedule which will delay it even more. The only way would be to defer_fn a couple of ms but that's really not good...

Creating a new event is also a good option, but the codebase is the same for confirm (used with eg delete (y/n) or change_cwd) and input (like add, copy etc).
I don't want to run the same event for those cases..

lua/neo-tree/ui/inputs.lua Outdated Show resolved Hide resolved
@pysan3
Copy link
Collaborator Author

pysan3 commented Mar 3, 2024

@cseickel Updated.

Regarding this change, I deprecated the enable_normal_mode_for_inputs option and added it to migrations.

As I wanted to give more information on how to migrate, I changed the migration module to be able to add descriptions in the buffer.

Besides, now bullet point for each change doesn't make much sense, so I changed it to use level 2 header ## Header 2 instead for each value.

image

@cseickel
Copy link
Contributor

cseickel commented Mar 3, 2024

It is definitely nice to be able to add a longer description to deprecations.

One thing I was considering is that I think it is more standard for plugins to have a start_in_insert_mode configuration for these situations. Maybe we should do the same and change the config option to start_inputs_in_insert_mode = false instead? Then we can keep the event for more advanced configuration but also have the simple one which is pretty standard.

What do you think?

@pysan3
Copy link
Collaborator Author

pysan3 commented Mar 3, 2024

I don't quite like when behavior before event handler execution changes by a different option in the config.

If you have an option to alter the behavior other than NEO_TREE_POPUP_INPUT_READY, I'd need to mention that in the docs with if ... otherwise ...

When people asks how to do xxx in the popup, I need to instruct; if you have config.start_inputs_in_insert_mode to true, then set this handler, and if false, set this handler.

I'd prefer to have the handler start consistently, either always from insert mode or always from normal mode.

@cseickel
Copy link
Contributor

cseickel commented Mar 3, 2024

When people asks how to do xxx in the popup, I need to instruct; if you have config.start_inputs_in_insert_mode to true, then set this handler, and if false, set this handler.

OK, fair enough. In that case, we stick with your idea.

Copy link
Contributor

@cseickel cseickel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay on this one. LGTM.

@cseickel cseickel merged commit bce4540 into nvim-neo-tree:main Mar 6, 2024
2 checks passed
@Sologala
Copy link

Sologala commented Mar 9, 2024

@pysan3 Hello, I update to release 3.19. The prompted NeoTree migrations reports some Error

E5108: Error executing lua ...m/lazy/neo-tree.nvim/lua/neo-tree/setup/deprecations.lua:18: Invalid option name: 'wrap'
stack traceback:
        [C]: in function 'nvim_buf_set_option'
        ...m/lazy/neo-tree.nvim/lua/neo-tree/setup/deprecations.lua:18: in function 'show_migrations'
        ...re/nvim/lazy/neo-tree.nvim/lua/neo-tree/command/init.lua:51: in function 'execute'
        ...re/nvim/lazy/neo-tree.nvim/lua/neo-tree/command/init.lua:169: in function '_command'
        [string ":lua"]:1: in main chunk

Do you know how to fix it or find more clues?

@pysan3
Copy link
Collaborator Author

pysan3 commented Mar 9, 2024

@Sologala
I'm so sorry: #1387

Please wait for the next release (which should happen right away).

You should see something like this so please follow the instructions.
image

@cseickel
Copy link
Contributor

cseickel commented Mar 9, 2024

The fix was just released as 3.20.

@Sologala
Copy link

@pysan3 @cseickel Thx for update, My neo-tree work delicately again!

@rrrix
Copy link
Contributor

rrrix commented Mar 13, 2024

@pysan3 I think you forgot to remove enable_normal_mode_for_inputs from defaults.lua. I'm still getting a deprecation/migration notification on every startup, despite no use of this key in any configuration file.

The only location I can find where enable_normal_mode_for_inputs is being set is in defaults.lua:

enable_normal_mode_for_inputs = false, -- Enable normal mode for input dialogs.

I believe this is because the removed(key, desc) checks for nil, and false is not nil.

local removed = function(key, desc)
local value = utils.get_value(config, key)
if type(value) ~= "nil" then
utils.set_value(config, key, nil)
migrations[#migrations + 1] =
string.format("The `%s` option has been removed.\n%s", key, desc or "")
end
end

$ lua
Lua 5.4.6  Copyright (C) 1994-2023 Lua.org, PUC-Rio
> foo = false
> foo ~= nil
true
> foo = nil
> foo ~= nil
false

Commenting out enable_normal_mode_for_inputs fixes the erroneous migration notification.

rrrix added a commit to rrrix/neo-tree.nvim that referenced this pull request Mar 13, 2024
@pysan3
Copy link
Collaborator Author

pysan3 commented Mar 14, 2024

My bad. Thank you @rrrix

@davawen
Copy link

davawen commented Mar 17, 2024

I may be missing a basic command, but can I make it such that I enter the popup in insert mode, but pressing escape puts me in normal mode (instead of leaving the popup) ?

That's the behavior I'm getting from other popups.

@pysan3
Copy link
Collaborator Author

pysan3 commented Mar 17, 2024

@davawen

Map "i", "<Esc>" to vim.cmd.stopinsert in the event. There's the exact example in :h neo-tree-events for you but you need to update to the latest main branch (not yet include in the latest release iirc).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants