feat: make ExecuteHostCommand accept closure#18018
Conversation
87d6614 to
5dc5a23
Compare
|
Is there a simpler way of doing this without exposing the internal of the |
5dc5a23 to
eb3e1f6
Compare
|
@fdncred let me know if you like direction it's going or I am doing something really stupid. |
|
wow, this is a lot of changes. I worry most about the repl.rs changes. I see a lot of code is just moved around into the run_command function. One thing that seems a little odd to me is storing the index as a string in ExecuteHostCommand and then parsing it back out again later. |
|
I'd also like to understand the use case and/or some examples of using a closure |
Yeah, most of the change is just moving the stuff inside the about passing a string, ExecuteHostCommand explicitly wants a string. So, i have no other choice. In an earlier commit, i had the whole commit serialized to a json. But i figured that was really inefficient. So, the index is passed as a string intead. We need something to tell apart which of the closures to run when someone triggers a keybind.
I added an example to the pr description. Previously we could only write that inside a string e.g. |
…18119) ## Description Previously, the `pre_prompt` and `env_change` hooks couldn't modify the commandline. Now they can. As a consequence, commands executed by keybindings (ExecuteHostCommand) also reset the commandline. I will probably wait for #18018 to fix that as that PR separates the `Signal::HostCommand` from the `Signal::Success` arm, so we can make the hooks work only if its not an `ExecuteHostCommand`. ## User-facing changes (Release notes) The `pre_prompt` and `env_change` hooks can now modify the commandline using the `commandline edit` command. Example: ```nushell $env.config.hooks.pre_prompt ++= [{ commandline edit "test" }] ``` ## Additional notes N/A
I am still not sure what is the correct data structure for this
|
I'm not a fan of these slotmap changes. It makes it harder to understand what's going on. |
I am still experimenting. This is not final. |
|
Good. I'd prefer not to make things more obscure. repl.rs is already complicated enough. |
Pull request nushell/reedline#1049 is needed to be merged before we can merge this PR. (merged)
This is currently just a prototype. I am open to ideas how we can improve it.
Release notes summary - What our users need to know
The
ExecuteHostCommandevent for keybindings now also accepts closure along side strings.