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

os.args() and passthru from CLI #102

Merged
merged 3 commits into from
Sep 18, 2023
Merged

os.args() and passthru from CLI #102

merged 3 commits into from
Sep 18, 2023

Conversation

edhemphill
Copy link
Contributor

i'll try to update this PR with some tests.

Copy link
Collaborator

@myzie myzie left a comment

Choose a reason for hiding this comment

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

This is looking good. Just left a couple minor questions/comments.

modules/os/os.go Outdated Show resolved Hide resolved
os/os.go Outdated Show resolved Hide resolved
os/virtual.go Outdated Show resolved Hide resolved
@edhemphill
Copy link
Contributor Author

updated. see what you think

@myzie
Copy link
Collaborator

myzie commented Sep 12, 2023

@edhemphill - this all looks good. Just this one little update to do:
https://github.com/risor-io/risor/pull/102/files#r1321470932

After that I think this is good to go. I'll give it a try just to sanity check.

Co-authored-by: Curtis Myzie <curtis.myzie@gmail.com>
Copy link
Contributor Author

@edhemphill edhemphill left a comment

Choose a reason for hiding this comment

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

good catch

Copy link
Collaborator

@myzie myzie left a comment

Choose a reason for hiding this comment

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

This looks good to me.

go run ./cmd/risor -c "os.args()" -- -foo --bar baz
[
  "-foo",
  "--bar",
  "baz"
]

One thing we should look at as a follow-up is what the shebang line should be now. What I had in there before doesn't seem to work, but I don't think that needs to hold up this PR.

Not working:

#!/usr/bin/env risor --

os.args()

@myzie myzie merged commit 0741e12 into risor-io:main Sep 18, 2023
2 checks passed
@edhemphill
Copy link
Contributor Author

yeah - i see what you're saying... will try to fix. It's not executing the script.

@edhemphill
Copy link
Contributor Author

edhemphill commented Sep 19, 2023

so in my testing this works:

test.tm:

#!/usr/bin/env /home/ed/work/risor/cmd/risor/risor 

os.args()

As well as some other variations with and without env. The issue is the script arg of the file with the #! is added after the -- so it gets passed to the risor script.

With the above, you can pass args to the risor script like:

 ./test.tm -- 1 2 3 

[
  "1",
  "2",
  "3"
]

To get around having to pass args behind a -- you could have a wrapper script for risor. As an example:

risor.sh:

#!/bin/bash

exec /path/to/risor "${1}" -- "${@:2}"

test.tm:

#!/path/to/risor.sh

os.args()
$ ./test.tm 1 2 3 

[
  "1",
  "2",
  "3"
]

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