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] Add callback support to Console\Question autocompleter#30997
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
[Console] Add callback support to Console\Question autocompleter#30997
Uh oh!
There was an error while loading.Please reload this page.
Conversation
MikkelPaulson commentedApr 8, 2019
The CI failures on this branch also appear to be occurring on the upstream master... |
fabpot left a comment
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.
LGTM. I've left some comments about CS.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
In order to enable more dynamic use cases such as word-by-wordautocomplete and path-based autocomplete, update the autocomplete logicof the Question object and its helper to accept a callback function.This function is called on each keystroke and should return an array ofpossibilities to present to the user.The original logic only accepted an array, which requiredimplementations to anticipate in advance all possible input values.This change is fully backwards-compatible, but reimplements the oldbehaviour by initializing a "dumb" callback function that always returnsthe same array regardless of input.
nicolas-grekas left a comment
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.
(I addressed Fabien's comment + did some changes while reviewing)
fabpot commentedApr 8, 2019
Thank you@MikkelPaulson. |
…ocompleter (Mikkel Paulson)This PR was merged into the 4.3-dev branch.Discussion----------[Console] Add callback support to Console\Question autocompleter| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes| BC breaks? | minor edge case, see below| Deprecations? | no| Tests pass? | yes (with expanded coverage)| Fixed tickets | N/A| License | MIT| Doc PR |symfony/symfony-docs#11349Autocompletion is a useful feature, but it's not always possible to anticipate every input the user could provide in advance. For instance, if we're allowing the user to input a path to a file, it's not practical to populate an array with every file and directory in the filesystem, but we can easily build a callback function that populates its suggestions based on the path already inputted.This change replaces the autocomplete logic that accepts an array of suggestions with an architecture that uses a callback function to populate suggestions in real time as the user provides input.The first commit adds a test class covering all methods of the `Question` object, while the second commit modifies the `Question` object to accept and store a callback function. The existing `[gs]etAutocompleterValues()` methods are preserved, but instead of being referenced directly from the `QuestionHelper`, they create and call their own callbacks to emulate the current behaviour.There is one edge case that is changed, as documented in the test: when a `Traversable` object is passed to `setAutocompleterValues()`, the return value of `getAutocompleterValues()` will be the unpacked (array) form of that object rather than the object itself. The unpacking is done lazily and cached on the callback function.Commits-------caad562 [Console] Add callback support to Console\Question autocompleter
This PR was merged into the master branch.Discussion----------Add description of autocompleter callbackDocumented feature added bysymfony/symfony#30997.Commits-------5cfc67e Add description of autocompleter callback
This PR was merged into the 4.3-dev branch.Discussion----------Improve test coverage from#30997| Q | A| ------------- | ---| Branch? | master| Bug fix? | yes (test enhancement against change on master)| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | enhancement for#30997| License | MIT| Doc PR | N/ATest coverage added in#30997 did a good job of validating previous behaviour, but didn't adequately cover the new callback logic. Added coverage for new methods on the Question object.Commits-------4693422 Improve test coverage from#30997
Uh oh!
There was an error while loading.Please reload this page.
Autocompletion is a useful feature, but it's not always possible to anticipate every input the user could provide in advance. For instance, if we're allowing the user to input a path to a file, it's not practical to populate an array with every file and directory in the filesystem, but we can easily build a callback function that populates its suggestions based on the path already inputted.
This change replaces the autocomplete logic that accepts an array of suggestions with an architecture that uses a callback function to populate suggestions in real time as the user provides input.
The first commit adds a test class covering all methods of the
Questionobject, while the second commit modifies theQuestionobject to accept and store a callback function. The existing[gs]etAutocompleterValues()methods are preserved, but instead of being referenced directly from theQuestionHelper, they create and call their own callbacks to emulate the current behaviour.There is one edge case that is changed, as documented in the test: when a
Traversableobject is passed tosetAutocompleterValues(), the return value ofgetAutocompleterValues()will be the unpacked (array) form of that object rather than the object itself. The unpacking is done lazily and cached on the callback function.