Skip to content

Conversation

@rustdevbtw
Copy link
Contributor

The unsafe install thing (that was added recently), creates a bit different script as you know.
Well, in that, the args that you pass to the program were being passed as a single argument ("$ARGS"). So, here's the simple one-line solution.

cc @jhheider

Signed-off-by: Rajdeep Malakar <rajdeepm.dev@gmail.com>
@coveralls
Copy link

Pull Request Test Coverage Report for Build 12028820129

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.9%) to 92.029%

Files with Coverage Reduction New Missed Lines %
src/utils/execve.ts 1 91.11%
Totals Coverage Status
Change from base Build 11988281915: 0.9%
Covered Lines: 1376
Relevant Lines: 1490

💛 - Coveralls

Copy link
Contributor

@jhheider jhheider left a comment

Choose a reason for hiding this comment

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

Shell splitting is the worst. Will this work with spaces in arguments, or does the code really need to use an array?

@rustdevbtw
Copy link
Contributor Author

Shell splitting is the worst. Will this work with spaces in arguments, or does the code really need to use an array?

Well, if you pass something like:

pandoc file.md -thtml

Using quotes will make it:

pandoc "file.md -thtml"

So, it wouldn't work.

Without $ARGS quoted:

pandoc file.md -thtml

So, yeah. It'll work with spaces in arguments with this solution, I suppose.

@mxcl mxcl force-pushed the main branch 21 times, most recently from b806c04 to 7c34462 Compare January 14, 2025 20:26
@mxcl
Copy link
Member

mxcl commented Jan 14, 2025

pkgx no longer installs, pkgm installs instead. Thank you and sorry!

If you want to install stubs we made a mash script where maybe this can go instead: https://github.com/pkgxdev/mash/blob/main/scripts/stub.sh

@mxcl mxcl closed this Jan 14, 2025
@mxcl
Copy link
Member

mxcl commented Jan 14, 2025

We could still merge this into the v1 branch tho.

@rustdevbtw
Copy link
Contributor Author

We could still merge this into the v1 branch tho.

Then I'll create a PR for the v1 branch, since it's sorta a bug on that one.

@rustdevbtw
Copy link
Contributor Author

We could still merge this into the v1 branch tho.

Then I'll create a PR for the v1 branch, since it's sorta a bug on that one.

By the way, I can't find the v1 branch. Where is it?

@jhheider
Copy link
Contributor

i don't believe one exists. there is a v1 tag at f28d931. we should have a branch there, in case there is demand for updates to v1.

@mxcl
Copy link
Member

mxcl commented Jan 15, 2025

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.

4 participants