Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Console] Improve UX on not found namespace/command#20869
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
linaori commentedDec 11, 2016
Seems a lot better! Is it possible to make a 2 column table (borderless) out of them so they align properly? |
Seldaek commentedDec 11, 2016
Yup, did it by hand because it's like 3 lines of code and the Table requires an output and all.. seemed overly complex for this.
|
76ecf4d to70b3e00Compare| $application->add(new \Foo2Command()); | ||
| $expectedMsg ="The namespace\"f\" is ambiguous.\nDid you mean one of these?\n foo\n foo1"; | ||
| $this->setExpectedException('Symfony\Component\Console\Exception\CommandNotFoundException',$expectedMsg); |
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 you can use::class there instead of the string? As the target is master this should be alright
| $exact =in_array($name,$commands,true); | ||
| if (count($commands) >1 && !$exact) { | ||
| $suggestions =$this->getAbbreviationSuggestions(array_values($commands)); | ||
| $usableWidth = ($this->terminal->getWidth() ?:60) -10; |
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.
getWidth always returns something, with a default of80
| $abbrevs =array_values($commands); | ||
| $maxLen =0; | ||
| foreach ($abbrevsas$abbrev) { | ||
| $maxLen =max(strlen($abbrev),$maxLen); |
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.
you should useHelper::strlen to deal with UTF-8 properly
| $abbrevs =array_map(function ($cmd)use ($commandList,$usableWidth,$maxLen) { | ||
| $abbrev =str_pad($cmd,$maxLen,'').''.$commandList[$cmd]->getDescription(); | ||
| returnstrlen($abbrev) >$usableWidth ?substr($abbrev,0,$usableWidth -3).'...' :$abbrev; |
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.
Helper::strlen
hason commentedDec 12, 2016
What about combine with PR#19282? |
Seldaek commentedDec 12, 2016
Fixed feedback (I had to add a Helper::substr too, I hope that's ok). @hason hmm yeah I guess if yours is merged it kinda makes this one obsolete anyway? I don't really have a preference, I guess this one is simpler but less powerful. |
fabpot commentedDec 13, 2016
Thank you@Seldaek. |
…eldaek)This PR was merged into the 3.3-dev branch.Discussion----------[Console] Improve UX on not found namespace/command| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | -| License | MITThis improves the DX/UX when you don't remember what a command is called.. Traditionally you get this message saying "command x is ambiguous (Y, Z or 6 more)" and if the one you are looking for is in the 6 more you are out of luck. You then have to run the console without arg again, get 50 commands displayed, then have to scroll up to find which one it is you meant.With this patch you get all suggestions always, even with description, so you can make an informed decision right away. See before/after on the screenshot below.Commits-------aae5fb1 Improve UX on not found namespace/command
This improves the DX/UX when you don't remember what a command is called.. Traditionally you get this message saying "command x is ambiguous (Y, Z or 6 more)" and if the one you are looking for is in the 6 more you are out of luck. You then have to run the console without arg again, get 50 commands displayed, then have to scroll up to find which one it is you meant.
With this patch you get all suggestions always, even with description, so you can make an informed decision right away. See before/after on the screenshot below.