-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Keep command out of program.args and add unknown to .action params #1048
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.
Thank you.
This PR now makes it impossible to detect whether any command has run, like the example in: #7 (comment) We want to print help when no command has run, and it doesn't really seem to be possible (process just exists without printing anything). Checking And on top of that |
@xPaw This PR does change the behaviour in an area that people have written code based on undocumented details of the previous behaviour, so will cause some breakages. Hopefully we can work out a more robust way to achieve the same behaviours, and where appropriate add migration steps to the Release Notes. I test
Please open a new issue with an example of what you are trying to do and some example code. |
Issue was already made in 2011 about this: #7, you could just re-open that. |
#7 was closed, with some up-voted example code supplied which tests Does that work for your case? |
This PR specifically "broke" testing My current "fix" was to use thelounge/thelounge@e09599a#diff-25f700bb76bbf957569bd288fbf7d328L63-R63 |
Ok, I see the problem. Not sure about best solution yet, but that is reasonable. |
I have added a migration tip to the Release Notes, but it might be an issue that needs a better solution. The detection of unrecognised commands and missing commands when using action handlers is a bit fragile. |
This reimplements #1030 after the move to Jest. See #1030 for description and discussion.