-
-
Notifications
You must be signed in to change notification settings - Fork 33
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
Support shorthand for package-manager
option
#57
Conversation
fix: do not fail command if grep has not found any version
Fix #45 |
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.
Thanks @corporateuser! Left a comment.
action.yml
Outdated
@@ -29,7 +29,7 @@ runs: | |||
len=`echo $INPUT_PM | wc -c` | |||
if [ $len -gt 1 ]; then | |||
PACKAGE_MANAGER=$(echo "$INPUT_PM" | grep -o '^[^@]*') | |||
VERSION=$(echo "$INPUT_PM" | grep -o '@.*' | sed 's/^@//') | |||
VERSION=$(echo "$INPUT_PM" | grep -o '@.*' || true | sed 's/^@//') |
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.
Ran a quick test locally and this seems to change the output of this command.
# current command:
echo "pnpm@14.7.0" | grep -o '@.*' | sed 's/^@//'
# output:
14.7.0
# PR change:
echo "pnpm@14.7.0" | grep -o '@.*' || true | sed 's/^@//'
# output:
@14.7.0
Grouping the central command does seem to work though:
VERSION=$(echo "$INPUT_PM" | grep -o '@.*' || true | sed 's/^@//') | |
VERSION=$(echo "$INPUT_PM" | (grep -o '@.*' || true) | sed 's/^@//') |
(Although I am 100% not a shell script expert so this may not be the correct fix)
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.
Oh, you're right
my command is equal to:
echo "pnpm@14.7.0" | grep -o '@.*' || { true | sed 's/^@//'; }
The proper command will be
echo "pnpm@14.7.0" | { grep -o '@.*' || true; } | sed 's/^@//'
Fix: proper grouping for grep
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.
Thanks @corporateuser — the updated command works for me locally 👍
package-manager
option
fix: do not fail command if grep has not found any version