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

Shell commands #547

Merged
merged 20 commits into from
Aug 31, 2021
Merged

Shell commands #547

merged 20 commits into from
Aug 31, 2021

Conversation

Omnikar
Copy link
Contributor

@Omnikar Omnikar commented Aug 3, 2021

Addresses #75.

I've added a config option for shell which would be set like shell = ["bash", "-c"], but the code does not currently use it because command functions are not able to access config currently. As such, the code is only able to fall back to cmd /C on Windows and sh -c otherwise. Currently, the command's stderr isn't handled.

  • Implement commands stated in this comment, in addition to $, which pipes each selection into the command, deleting the selection if the command exits with a nonzero exit status.
  • Shell command completions
  • Allow the user to configure the shell to use in config.toml.

@Omnikar
Copy link
Contributor Author

Omnikar commented Aug 3, 2021

What is causing the check failure?

@archseer
Copy link
Member

archseer commented Aug 4, 2021

I think it's just a github failure, re-running tests now.

helix-term/src/commands.rs Outdated Show resolved Hide resolved
helix-term/src/keymap.rs Outdated Show resolved Hide resolved
helix-term/src/config.rs Outdated Show resolved Hide resolved
@archseer
Copy link
Member

archseer commented Aug 4, 2021

I think on POSIX systems we can follow Kakoune's lead: it looks for a KAKOUNE_POSIX_SHELL variable or else falls back to sh -c.

helix-term/src/commands.rs Outdated Show resolved Hide resolved
helix-term/src/commands.rs Outdated Show resolved Hide resolved
helix-term/src/commands.rs Outdated Show resolved Hide resolved
@sudormrfbin sudormrfbin linked an issue Aug 6, 2021 that may be closed by this pull request
helix-term/src/commands.rs Outdated Show resolved Hide resolved
@Omnikar
Copy link
Contributor Author

Omnikar commented Aug 22, 2021

Should I do anything with the command's stderr?

@pickfire
Copy link
Contributor

Should I do anything with the command's stderr?

Maybe you can log it?

@Omnikar
Copy link
Contributor Author

Omnikar commented Aug 25, 2021

Instead of unwrapping potential errors, I think I might want to log an error, display a message, and return?

helix-term/src/commands.rs Outdated Show resolved Hide resolved
helix-term/src/commands.rs Outdated Show resolved Hide resolved
`shell` will no longer panic if:
  * The user-configured shell cannot be found
  * The shell command does not output UTF-8
@Omnikar Omnikar requested a review from cessen August 26, 2021 23:49
@Omnikar Omnikar marked this pull request as ready for review August 30, 2021 16:50
@archseer archseer merged commit e772808 into helix-editor:master Aug 31, 2021
@Omnikar Omnikar deleted the shell-cmds branch August 31, 2021 09:17
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.

External command support
5 participants