nushell / reedline

A feature-rich line editor - powering Nushell
https://docs.rs/reedline/
MIT License
547 stars 151 forks source link

ExecuteHostCommand doesnt run the command #524

Open alexzanderr opened 1 year ago

alexzanderr commented 1 year ago

hello

im trying to run a simple command that returns output

i've modified the demo example by adding a new keybiding:

fn add_extra_keybindings(keybindings: &mut Keybindings) {
    // This doesn't work for macOS
    keybindings.add_binding(
        KeyModifiers::CONTROL,
        KeyCode::Char('f'),
        ReedlineEvent::ExecuteHostCommand("echo 'its working'".into()),
        // doesnt work
        // ReedlineEvent::ExecuteHostCommand(
        //     "/usr/bin/exa --all --icons".into(),
        // ),
    );
}

here i've setup the binding ctrl+f to run the command echo 'its working'

what im expecting: reedline to print the output of the command in the prompt, just like a real shell. isnt that the expected behaviour?

what im getting instead is:

Our buffer: echo 'its working'
prompt 〉

that indicates that the buffer state after running the command is the command itself.

i want the output to be printed in the terminal or maybe run another inner prompt that gets input which then will exit the result into reedline.

what am i missing?

i've looked a little in the source: engine.rs

            ReedlineEvent::ExecuteHostCommand(host_command) => {
                // TODO: Decide if we need to do something special to have a nicer painter state on the next go
                Ok(EventStatus::Exits(Signal::Success(host_command)))
            }

but im not seeing any Command::new().arg() ... .spawn() etc, rusty stuff here; just a signal success with the command as string

is this code running the command? seems to not.

thanks in advance.

more advanved extra topic: how can i run an external shell command that prompts a UI, for example skim the fuzzy finder inside reedline? (but that is after im solving the first problem)

alexzanderr commented 1 year ago

ok i've played a little bit

this theoretical simulation of how to run an example command works:

            ReedlineEvent::ExecuteHostCommand(host_command) => {
                // TODO: Decide if we need to do something special to have a nicer painter state on the next go
                use std::process::Command;
                let out = Command::new("sh")
                    .arg("-c")
                    .arg(host_command)
                    .output()
                    .unwrap()
                    .stdout;
                let out = String::from_utf8(out).unwrap();
                Ok(EventStatus::Exits(Signal::Success(out)))
            },

my concern is: should that be there in the first place? my guess: this is not implemented yet?

but wait its implemented in nushell how come here its not implemented.

sholderbach commented 1 year ago

We added ReedlineEvent::ExecuteHostCommand to be shell/programming language agnostic. It just emits the code that a user would have typed on a keypress. The responsibility of executing/evaluating that is on the hosting application (nushell or any other interpreter). We could split up the Signal to make that more clear but I think including a way to execute arbitrary shell code is definitely not a goal for reedline.

alexzanderr commented 1 year ago

so from what im understanding ReedlineEvent::ExecuteHostCommand doesnt do anything, just returns the command back, right?

alexzanderr commented 1 year ago

The responsibility of executing/evaluating that is on the hosting application (nushell or any other interpreter)

ok. what should i write to evaluate the command without modifying the source code of reedline? because i havent seen anywhere an example of running a host command

alexzanderr commented 1 year ago

but I think including a way to execute arbitrary shell code is definitely not a goal for reedline.

indeed. i agree. expanding the project with extra functionality will get out of the original scope, becoming more general.

alexzanderr commented 1 year ago

i think i see what are you referring to.

by saying this:

The responsibility of executing/evaluating that is on the hosting application

you mean that actual execution of the command should be handled here in the loop

    loop {
        let sig = line_editor.read_line(&prompt)?;
        match sig {
            Signal::Success(buffer) => {
                println!("We processed: {}", buffer);
            },
            Signal::CtrlD | Signal::CtrlC => {
                println!("\nAborted!");
                break Ok(());
            },
        }
    }

after the keybinding

    keybindings.add_binding(
        KeyModifiers::CONTROL,
        KeyCode::Char('f'),
        ReedlineEvent::ExecuteHostCommand("echo 'its working'".into()),
    );

sent that specific information to run the command.

so basically ExecuteHostCommand its just a signal (ofc, that's why its called an event) for the user to handle in the mainloop. right?

cactusdualcore commented 1 year ago

It feels to me like ExecuteHostCommand is a misleading name if it does not execute anything. It could be renamed to what it actually does, which is passing a prefilled line to the using app. Maybe TextPassthrough or LinePassthrough?