[Console] Improve UX on not found namespace/command #20869
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Seems a lot better! Is it possible to make a 2 column table (borderless) out of them so they align properly? |
|
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 to
70b3e00
Compare
Simperfit
approved these changes
Dec 11, 2016
theofidry
approved these changes
Dec 11, 2016
| public function testFindAmbiguousNamespace() | ||
| { | ||
| $application = new Application(); | ||
| $application->add(new \BarBucCommand()); | ||
| $application->add(new \FooCommand()); | ||
| $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.
maybe you can use ::class there instead of the string? As the target is master this should be alright
| @@ -559,9 +559,20 @@ public function find($name) | |||
|
|
|||
| $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.
getWidth always returns something, with a default of 80
| $abbrevs = array_values($commands); | ||
| $maxLen = 0; | ||
| foreach ($abbrevs as $abbrev) { | ||
| $maxLen = max(strlen($abbrev), $maxLen); |
There was a problem hiding this comment.
you should use Helper::strlen to deal with UTF-8 properly
| $abbrevs = array_map(function ($cmd) use ($commandList, $usableWidth, $maxLen) { | ||
| $abbrev = str_pad($cmd, $maxLen, ' ').' '.$commandList[$cmd]->getDescription(); | ||
|
|
||
| return strlen($abbrev) > $usableWidth ? substr($abbrev, 0, $usableWidth - 3).'...' : $abbrev; |
|
What about combine with PR #19282? |
|
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. |
|
Thank you @Seldaek. |
fabpot
added a commit
that referenced
this pull request
Dec 13, 2016
…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 | MIT 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.  Commits ------- aae5fb1 Improve UX on not found namespace/command
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.