-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Console] Do not leak hidden console commands #33412
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
Shouldn't we do this on 4.4, to not break scripts that use a short alias into the wild? |
Agreed, that's probably a good idea as this issue is rather a cosmetic one. Do you want me to rebase on 4.4?* *) btw: Should I add return type hints for new functions in this case even though the other functions inside |
Please rebase for 4.4 yes, and you can add types for sure. |
This part should be for 3.4. |
@chalasr How would that work without the other changes? Assume you're input is ambiguous and you get a suggestion with two commands where one of them is hidden. Now stripping the hidden one would return a single command saying that it is ambiguous. Isn't that worse that leaving it as it is? |
rebased for 4.4 fabbot reporting about CS seems unrelated to my changes (it's about unsorted use statements). |
fabbot patch applied in 61dad16 |
I accidentally rebased against the wrong branch first and then against 4.4 again. |
Done. |
@chalasr we need your help to unlock here (you can also approve as is :) ) |
(rebase needed btw, to see tests green) |
8573af8
to
24a98d7
Compare
PR rebased + added 1 commit to turn the CommandNotFoundException into a deprecation notice in case one uses an abbreviation for finding an hidden command. |
…rnatives (m-vo) This PR was merged into the 3.4 branch. Discussion ---------- [Console] Do not include hidden commands in suggested alternatives | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #33398 | License | MIT | Doc PR | n/a Partially backports #33412 on 3.4, avoids leaking the names of hidden commands in suggested alternatives on command not found. Commits ------- 8a9d173 Do not include hidden commands in suggested alternatives
38e67fd
to
92b5a29
Compare
92b5a29
to
f340633
Compare
Thank you @m-vo. |
This PR was merged into the 4.4 branch. Discussion ---------- [Console] Do not leak hidden console commands | Q | A | ------------- | --- | Branch? | 4.4 (updated) | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #33398 | License | MIT This PR attempts to fix hidden console commands to be leaked when interacting with the console. These are the changes: * Hidden commands won't be shown anymore in the list of commands in a namespace as well as the list of suggestions ("Did you mean...") for invalid or ambiguous commands. * Hidden commands therefore now need to be always entered with their full name. * If an abbreviated command is entered that was previously ambiguous with (only) hidden commands, it's now executed directly (not ambiguous anymore). Side note: When implementing the tests & changes I realized that `Application->get()` isn't side effect free (when redirecting to the help command) and behaves differently when called multiple times. It therefore must not be used from inside `find()`. Maybe we should change this? Here are the relevant bits: https://github.com/symfony/symfony/blob/f71f74b36a80227d3e68f1b65b1f1d9b42fa9952/src/Symfony/Component/Console/Application.php#L495-L502 Commits ------- f340633 [Console] Deprecate abbreviating hidden command names using Application->find()
This PR attempts to fix hidden console commands to be leaked when interacting with the console.
These are the changes:
Side note: When implementing the tests & changes I realized that
Application->get()
isn't side effect free (when redirecting to the help command) and behaves differently when called multiple times. It therefore must not be used from insidefind()
. Maybe we should change this? Here are the relevant bits:symfony/src/Symfony/Component/Console/Application.php
Lines 495 to 502 in f71f74b