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

No operations in until will be handled after an EditCommand #644

Open
101313 opened this issue Oct 6, 2023 · 6 comments
Open

No operations in until will be handled after an EditCommand #644

101313 opened this issue Oct 6, 2023 · 6 comments
Labels
A-EditOps Area: core editing operations (shared between keybindings) A-KeybindingEmacs Area: Handling of default (Emacs style) keybindings A-ViKeybinding Area: Vi(m) keybinding support

Comments

@101313
Copy link

101313 commented Oct 6, 2023

Describe the bug

This has showed up in a couple of occasions already, if I understand correctly this is incorrect behavior, please correct me if I'm wrong.

{
            name: move_to_line_end_or_take_history_hint
            modifier: control
            keycode: char_e
            mode: [emacs, vi_normal, vi_insert]
            event: {
                until: [
                    {send: historyhintcomplete}
                    {edit: movetolineend}
                    {send: openeditor} #TODO find out why this doesn't work
                ]
            }
        }

For instance, it should reach the third case when pressing ctrl+e on an empty buffer but it does not.

Or, similar case:

{
            name: move_to_line_start
            modifier: control
            keycode: char_a
            mode: [emacs, vi_normal, vi_insert]
            event: {
              until: [
                {edit: movetolinestart}
                {edit: movetostart} #TODO find out why this doesn't work
              ]
            }
        }

When editing a multi-line command, hitting ctrl+a brings you to the beginning of the line, but all consequent invocations return true as well even if there is no actual action performed.

How to reproduce

Try any of the aforementioned keybindings.

Expected behavior

Upon reaching the beginning or end of the line, subsequent calls should return false instead of true.

Screenshots

No response

Configuration

key value
version 0.85.0
branch
commit_hash
build_os linux-aarch64
build_target aarch64-alpine-linux-musl
rust_version rustc 1.72.1 (d5c2e9c34 2023-09-13) (Alpine Linux 1.72.1-r0)
cargo_version cargo 1.72.1
build_time 2023-09-20 19:52:50 +00:00
build_rust_channel release
allocator mimalloc
features default, sqlite, trash, which, zip
installed_plugins

Additional context

I've considered opening the issue on the reedline repo instead but I wasn't sure. If that's a duplicate or its more appropriate to post it elsewhere, please let me know.

@fdncred
Copy link
Collaborator

fdncred commented Oct 6, 2023

On both of your keybindings you're using until. until means "run these things in order until one works". I'm wondering if that's what you're meaning to do.

Here's how you might change the first one.

  {
    name: move_to_line_end_or_take_history_hint
    modifier: control
    keycode: char_e
    mode: [emacs, vi_normal, vi_insert]
    event: [
        {send: historyhintcomplete}
        {edit: movetolineend}
        {send: openeditor} 
    ]
  }

You can do something similar with your other one too.

@101313
Copy link
Author

101313 commented Oct 6, 2023

On both of your keybindings you're using until. until means "run these things in order until one works". I'm wondering if that's what you're meaning to do.

I think this is indeed what I would like to do. Unless you can conditionally sequence commands in other ways.

In essence, inside until commands take precedence by their order. In my example that would translate, if there is a suggestion take it, otherwise movetolineend and if that fails as well, which should when you're at the end already, then you may open the editor.

Not sure what the proposed solution does exactly, I would assume executing the commands, one after the other, but I don't think I've ever seen that being done outside of an until. At least, that is the case in the default config.

@fdncred
Copy link
Collaborator

fdncred commented Oct 6, 2023

ok, I guess the problem is that movetolineend will never fail, same with movetolinestart, so I'm guessing you'd never get to the last command in the until.

@101313
Copy link
Author

101313 commented Oct 7, 2023

ok, I guess the problem is that movetolineend will never fail, same with movetolinestart, so I'm guessing you'd never get to the last command in the until.

Yes, pretty much. I think its more reasonable for the other way around besides being more practical.

@sholderbach
Copy link
Member

Yes our EditCommand operations are all infallible at the moment so they will never say that they were not handled.

Here all roads lead to EventStatus::Handled.

reedline/src/engine.rs

Lines 1148 to 1190 in b1344f6

ReedlineEvent::Edit(commands) => {
self.run_edit_commands(&commands);
if let Some(menu) = self.menus.iter_mut().find(|men| men.is_active()) {
if self.quick_completions && menu.can_quick_complete() {
match commands.first() {
Some(&EditCommand::Backspace)
| Some(&EditCommand::BackspaceWord)
| Some(&EditCommand::MoveToLineStart) => {
menu.menu_event(MenuEvent::Deactivate)
}
_ => {
menu.menu_event(MenuEvent::Edit(self.quick_completions));
menu.update_values(
&mut self.editor,
self.completer.as_mut(),
self.history.as_ref(),
);
if let Some(&EditCommand::Complete) = commands.first() {
if menu.get_values().len() == 1 {
return self
.handle_editor_event(prompt, ReedlineEvent::Enter);
} else if self.partial_completions
&& menu.can_partially_complete(
self.quick_completions,
&mut self.editor,
self.completer.as_mut(),
self.history.as_ref(),
)
{
return Ok(EventStatus::Handled);
}
}
}
}
}
if self.editor.line_buffer().get_buffer().is_empty() {
menu.menu_event(MenuEvent::Deactivate);
} else {
menu.menu_event(MenuEvent::Edit(self.quick_completions));
}
}
Ok(EventStatus::Handled)
}

To allow more until shenanigans, we would need to make each of the edit operations explicit about their return status. Then you could reorder them more freely.

Moving this over to reedline as the changes would need to happen here.

@sholderbach sholderbach transferred this issue from nushell/nushell Oct 10, 2023
@sholderbach sholderbach added A-KeybindingEmacs Area: Handling of default (Emacs style) keybindings A-ViKeybinding Area: Vi(m) keybinding support A-EditOps Area: core editing operations (shared between keybindings) labels Oct 10, 2023
@sholderbach sholderbach changed the title Incorrect return of some commandline edit commands No operations in until will be handled after an EditCommand Oct 10, 2023
@Abdillah
Copy link
Contributor

Abdillah commented Oct 20, 2023

I have two related issues open regarding until too: #603 and #604.

If time suffice and no one taking, this is a one-strike issue after my #608 to pull it off.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-EditOps Area: core editing operations (shared between keybindings) A-KeybindingEmacs Area: Handling of default (Emacs style) keybindings A-ViKeybinding Area: Vi(m) keybinding support
Projects
None yet
Development

No branches or pull requests

5 participants