-
-
Notifications
You must be signed in to change notification settings - Fork 21.5k
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
Command line argument '--version' returns exit code 0 instead of 255 #62550
Conversation
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.
Make sense, should also be added to --help
. Other goto error;
cases seem to all be proper invalid parameter errors so they're fine.
I think the current behavior is fine, most software seems to have human readable
|
…stead of 255 Allows to detect whether those commands executed successfully, which makes integration with shell scripts/CI/bindings straightforward.
94a0f3a
to
e3a8edf
Compare
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.
Looks good!
Thanks! |
Thanks for the quick feedback! 🙂 |
Edit: Reverted from |
This introduced a bug:
And doesn't actually achieve what was intended, since it still returns a failure code but further down. I didn't pay close attention but this PR actually makes |
…ode 0 instead of 255" This reverts commit 9e165a8. See #62550 (comment).
Cherry-picked for 3.5. Edit: Seems like I messed up locally and forgot to push this back then. Fixed on 2022-07-08 with da78e92. |
…ode 0 instead of 255" This reverts commit 9e165a8. See godotengine#62550 (comment).
Allows to detect whether printing the version was successful and makes integration with shell scripts straightforward.
Minor thing:
--version
is usually read by tools (CI, shell scripts, bindings) and at the moment, an extra newline is being printed, which requires trimming. On the other hand, it makes things a bit more readable for humans. Let me know if the current behavior is good as it is.