-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[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 our terms of service and privacy 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
Conversation
The CI failures on this branch also appear to be occurring on the upstream master... |
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.
In order to enable more dynamic use cases such as word-by-word autocomplete and path-based autocomplete, update the autocomplete logic of the Question object and its helper to accept a callback function. This function is called on each keystroke and should return an array of possibilities to present to the user. The original logic only accepted an array, which required implementations to anticipate in advance all possible input values. This change is fully backwards-compatible, but reimplements the old behaviour by initializing a "dumb" callback function that always returns the same array regardless of input.
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)
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#11349 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 `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 callback Documented feature added by symfony/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/A Test 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
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
Question
object, while the second commit modifies theQuestion
object 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
Traversable
object 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.