-
Notifications
You must be signed in to change notification settings - Fork 222
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
[bug] melos run
args only work in single line scripts
#232
Comments
Doh, need to tinker with this one since the current implementation just passes them along as additional args at the end of the script, maybe I need to explicitly define positional args as envs myself |
@Salakar Yeah, I've tried it for a little bit myself, and it's trickier than it seems 😜 In bash, functions aren't first class citizens, so you can't simply define an anonymous one and call it immediately. However, one idea I had was, wrap any script you get in the following: "_run_melos_script() { $scriptPayload ; }" Then simply run the script and append the arguments final scriptFunction = "_run_melos_script() { $scriptPayload ; }"
await _startProcess("$scriptFunction && _run_melos_script ${restArgs.map((arg) => '"$arg"').join()}") It's a bit rough, but I think it might do the job. I'll do some experimenting. Lmkwyt 😄 |
I'm fine with it being a bit rough if it works, but it would also need to work on Windows too 😭 |
I wonder if this would also fix an issue I'm having trying to pass in a variable: I'm trying to use this command in a melos script: melos exec --since=<commit hash> -- flutter test where the commit hash could be passed in dynamically like: melos exec --since=$1 -- flutter test I can't see any way to do this currently from the documentation. I've tried running (in various forms, using the Melos script: args:
description: args test
run: melos exec --dir-exists=$1 flutter test Melos command: melos args test which produces: $ melos exec
└> flutter test test
└> RUNNING (in 5 packages) |
I have implemented a solution to this issue and submitted it as a pull request. #525 |
When will this feature be released finally? I really need it. |
Extra script args never quite worked as described in this issue. The test for extra args was a bit misleading, which has been fixed in #540. You were never able to access the extra args with I think the current behavior is actually what we want, though. It is simple and works on all platforms. Adding support for real placeholders that can be used anywhere in the script in a way that works on all platforms leads us down a path where we create our own little shell language for scripts. Generally, I feel like we should not encourage people to use multi-line scripts because they routinely forget to use It's typically better to put the implementation of more complex scripts into a separate file (e.g. a shell script, or better yet, a Dart script). Then you can do all the parsing and processing of extra args there. |
Hm, before running, can't we write the run script to a temp folder and execute it from the file? something like |
Hi there! 👋🏻
@Salakar graciously added the functionality to run
melos run
scripts with additional arguments in #231. 😄Description
Looking at the tests and trying it out myself, it works fine in single line scripts.
Output:
Actual behavior
However, using a multi-line string in YAML to define a script breaks this behavior.
Example 1
Output:
Example 2
Output:
Expected behavior
I believe arguments should be handled independently of the format of the script payload provided to Melos, just like when executing a
some_script.sh first second third
would parse$1
,$2
, etc. correctly.If you need any additional info or help, feel free to reach out! 😄
Great job y'all 👍🏻
The text was updated successfully, but these errors were encountered: