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

Support shell() #2047

Merged
merged 11 commits into from
May 20, 2024
Merged

Support shell() #2047

merged 11 commits into from
May 20, 2024

Conversation

gyreas
Copy link
Contributor

@gyreas gyreas commented May 16, 2024

This PR add support for the shell(), which expands to the output of running its given command.

Example:

set shell := ["fish", "-c"]
v := shell("
set p $argv
for i in $p
	echo "$i"
end
",  "Nightmares", "on", "Elms", "Street")

_:
  @echo "{{ v }}"

output:

Nightmares
on
Elms
Street

@casey
Copy link
Owner

casey commented May 17, 2024

This seems really nice! What do you think about getting rid of the name argument for now? Since a lot of uses of this function won't use it, it seems unfortunate to force users to pass an additional dummy argument if they want to pass arguments. I'd like to enable it in the future with a dedicated named argument syntax, i.e., shell(CMD, name=NAME, ARGS), which is generally useful and can be done in a future PR.

@casey
Copy link
Owner

casey commented May 17, 2024

Also, this should be documented in the readme.

@gyreas
Copy link
Contributor Author

gyreas commented May 17, 2024

Alright. I clean stuff up, I think I have a solution to remove that inconvenience. Thanks

@gyreas gyreas force-pushed the shell branch 2 times, most recently from c077e73 to 064f7c1 Compare May 17, 2024 12:58
@casey
Copy link
Owner

casey commented May 18, 2024

I think this should be refactored to use the same code as Evaluator::run_backtick. run_backtick does a few additional things, like setting the working directory, exporting environment variables, and the destination of stderr.

To make this easier, I just landed #2048, which gets rid of FunctionContext and instead just passes the Evaluator, since all fields on FunctionContext are just taken from Evaluator anyways.

The signature of run_backtick can be changed to something like:

fn run_command(&self, command: &str, args: &[String], token: Option<&Token<'src>>) -> RunResult<'src, String> {

Which produces an Error::Backtick when token is present, and an Error::FunctionCall when it isn't.

@casey casey enabled auto-merge (squash) May 20, 2024 00:22
@casey casey merged commit c6612de into casey:master May 20, 2024
5 checks passed
@casey
Copy link
Owner

casey commented May 20, 2024

Nice, LGTM!

@gyreas gyreas deleted the shell branch May 20, 2024 01:40
@gyreas
Copy link
Contributor Author

gyreas commented May 20, 2024

Thank you!

@casey
Copy link
Owner

casey commented May 20, 2024

Thank you! shell() is a great addition!

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.

2 participants