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

fix: split in cli args before calling ni #233

Closed
wants to merge 1 commit into from

Conversation

patak-dev
Copy link
Member

ni started to respect the args passed to getCommand if they have spaces, wrapping them in quotes:

I think this should do the trick for us. We could later migrate to a library to do this, for example, -- isn't respected. We don't need support for it in vite-ecosystem-ci config at this point though.

@@ -146,6 +146,13 @@ export async function setupRepo(options: RepoOptions) {
}
}

function splitInCliArgs(task: string) {
return task
.split(/("[^"]+"|[^\s"]+)/gim)
Copy link
Collaborator

@dominikg dominikg Jul 31, 2023

Choose a reason for hiding this comment

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

this can break down if you wanted to pass a literal \". Another way to support aguments for commands would be to require configs to pass an array of arrays eg
test:[['test:ci','arg-1,'arg2 with spaces'],['test:e2e','another arg']] then we can take those arrays to ni directly if the first element is a script, otherwise send it to the shell with quotes.

Copy link
Member Author

Choose a reason for hiding this comment

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

Implemented this here so you can check and merge what looks better:

I'm actually fine with either approach. #234 isn't as clean as it seemed at the end though.

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