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

Add run-shell-command for Commands #1682

Merged
merged 13 commits into from
May 2, 2022

Conversation

hayashikun
Copy link
Contributor

I added a command to run a shell command as discussed in #1541.
Just running, no piping, no inserting/appending

Output is displayed in the info area.

_d_helix.2022-02-20.14-34-08_Trim.mp4

helix-term/src/commands.rs Outdated Show resolved Hide resolved
helix-term/src/commands.rs Outdated Show resolved Hide resolved
helix-view/src/info.rs Outdated Show resolved Hide resolved
@hayashikun hayashikun changed the title Add run_shell_command for Commands Add run-shell-command for Commands Feb 21, 2022
@hayashikun hayashikun requested a review from archseer February 21, 2022 11:18
@flukejones
Copy link
Contributor

I was hoping something like this would come around.

Is it possible to scroll the output with keys?

@hayashikun
Copy link
Contributor Author

Now the Info component shows the result, and there is no way to scroll because Info doesn't have handle_event.
Implementing handle_event into Info is not a good way.

Popup can scroll (ctrl-u, ctrl-d), so you can scroll by using Popup instead of the Info.
But Popup is rendered at the cursor, like hover (space k).

Or add a new component that can scroll.

@archseer
Copy link
Member

Popup is the correct component to use, it has a set_position function that can be used to place it somewhere else than the cursor.

@hayashikun
Copy link
Contributor Author

agree
I changed it to use Popup.

Because scroll doesn't work in ui::Text, ui::Markdown and format!("```sh\n{}\n```", output) is used.
It's somewhat adhoc, but it definitely works.

asciicast

@IsotoxalDev
Copy link
Contributor

It would also be nice if you can have an option where no output is there. To run the command in the background

@hayashikun
Copy link
Contributor Author

To suppress the output, you can redirect to /dev/null.
Is this not enough?

Also, I noticed that running shell (:sh, pipe, pipe-to, ...) will stop the editor even if & is used.
When you press Alt-| and run sleep 10 &, the editor will be freeze for 10 seconds.

To run time-consuming tasks in the background, it would be better to fix the freezing problem.

@IsotoxalDev
Copy link
Contributor

Why not run the shell commands in a seperate thread?
Ys the redirecting to /dev/null is enough

@hayashikun
Copy link
Contributor Author

Asynchronous shell execution is not so tough with tokio::process::Command, but shell_impl is used in |, |Alt-|, !, Alt-!, $, and it should be executed synchronously except in Alt-|.
It would be better to create a separate issue/PR for asynchronous execution and discuss it there.

cx.editor.set_error("Command failed");
}

if !output.is_empty() {
Copy link
Contributor

Choose a reason for hiding this comment

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

So we don't show anything if there is no output? How does user know if it finished running?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now the shell command is executed synchronously, the user will know it's finished when it releases from the freeze.
I think it's one way to display a message like "Command successes" in the status area if success.

text,
width: body.lines().map(|l| l.width()).max().unwrap() as u16,
height: body.lines().count() as u16,
text: body,
Copy link
Contributor

@pickfire pickfire Feb 27, 2022

Choose a reason for hiding this comment

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

Suggested change
text: body,
text,

Using text here is clearer I think? But probably need to change the above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see.
But the information is now displayed in Popup, should I revert info.rs change?

Choose a reason for hiding this comment

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

Hey is this going into the main branch any time soon?

@hayashikun
Copy link
Contributor Author

Sorry for the late response.
I merged master, reverted info.rs, and "Command successes" is displayed if success.

@IsotoxalDev
Copy link
Contributor

cool

helix-view/src/info.rs Outdated Show resolved Hide resolved
@IsotoxalDev
Copy link
Contributor

Can't it be merged now?

@archseer archseer merged commit f85f0b7 into helix-editor:master May 2, 2022
@nixpulvis
Copy link

nixpulvis commented Oct 11, 2022

:sh yes seems to freeze the terminal. I was kinda hoping this would work. I'm also really not a fan of the popup output for the results.

I'm just trying Helix for the first time right now so there's a lot I don't know and terminology I'm probably wrong about, but... here are my general desires around this.

I would like a new pane/split within the current window to open, connected to the streaming IO for the process (or at least the output).

Basically, I want this to feel more like Vim's ! operator. In fact, ! compatibility (whatever that means exactly) would be pretty amazing. For example, I frequently find myself doing this: :w !sudo tee % which is a nice trick to save a file as root.

Here are a couple other use cases for asynchronous shell commands in the editor that come to mind, though the list could be nearly endless.

  • Running a web server quickly: ./server.sh (this is particularly nice when the server hot reloads the code)
  • tail -f ... staring at logs
  • Just running the build itself make, cargo build, etc. I can't tell you how many times I've run !make in vim, but it's a lot

I think there's a lot of potential here. I especially love the dropdown/ups for auto-completion being prevalent, but I get the sense that a lot of these features need some UX work and polish.

I hope this is helpful.

@flukejones
Copy link
Contributor

@nixpulvis I know it's not really what you might be looking for here, but maybe you could try using https://zellij.dev/ ? I find it very very good and easy to use.

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.

8 participants