-
Notifications
You must be signed in to change notification settings - Fork 30
Proper support for shell input #189
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
Conversation
Signed-off-by: kpenfound <[email protected]> assemble dagger command in new step with output Signed-off-by: kpenfound <[email protected]> fixing some inline bash Signed-off-by: kpenfound <[email protected]> wrap conditional inputs in quotes Signed-off-by: kpenfound <[email protected]> use cat for heredoc script file Signed-off-by: kpenfound <[email protected]> missing space in heredoc Signed-off-by: kpenfound <[email protected]> add tab ignore and dagger shebang Signed-off-by: kpenfound <[email protected]> use toJson and jq to safely write the shell input to a file Signed-off-by: kpenfound <[email protected]> put DAGGER_COMMAND in a variable Signed-off-by: kpenfound <[email protected]> missing semicolon for the dagger command Signed-off-by: kpenfound <[email protected]> safely evaluate if shell is set Signed-off-by: kpenfound <[email protected]> extra > in shell file write Signed-off-by: kpenfound <[email protected]> add some logging Signed-off-by: kpenfound <[email protected]> strip extra newline from jq when shell is not set Signed-off-by: kpenfound <[email protected]> debugging Signed-off-by: kpenfound <[email protected]> toJSON and jq will send null instead of empty string with no input Signed-off-by: kpenfound <[email protected]> safely handle all inputs Signed-off-by: kpenfound <[email protected]> did i fix the wrong thing? Signed-off-by: kpenfound <[email protected]> change up the approach a bit for an easier diff Signed-off-by: kpenfound <[email protected]> fix shell test Signed-off-by: kpenfound <[email protected]> single quotes around shell input Signed-off-by: kpenfound <[email protected]> try with piped input Signed-off-by: kpenfound <[email protected]> try single quotes again Signed-off-by: kpenfound <[email protected]> debug Signed-off-by: kpenfound <[email protected]> debug Signed-off-by: kpenfound <[email protected]> fix shell test Signed-off-by: kpenfound <[email protected]> does shell emit a newline? Signed-off-by: kpenfound <[email protected]> remove the trailing newline if its in stdout Signed-off-by: kpenfound <[email protected]> break the trailing output test Signed-off-by: kpenfound <[email protected]> fix shell test Signed-off-by: kpenfound <[email protected]> missed ! Signed-off-by: kpenfound <[email protected]> add comment about weird test Signed-off-by: kpenfound <[email protected]>
.github/workflows/test.yml
Outdated
run: | | ||
target='${{ steps.test-shell.outputs.output }}' | ||
result='hello world! | ||
' # shell appears to emit an extra newline compared to call |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I suspect this is because echo
includes a newline (while the call to shykes/daggerverse
just directly returns a string without the newline).
We could either use the same test for both, or just use -n
on the echo
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah let me try with -n
version: "latest" # semver x.y.z | ||
``` | ||
### `dagger shell` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could make this the first option?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe, i suspect call
will still be the standard approach in ci configurations because of the named args
Signed-off-by: kpenfound <[email protected]>
Closes #180