8000 Update prediction interface to provide additional feedback to a predictor plugin by daxian-dbw · Pull Request #15421 · PowerShell/PowerShell · GitHub
[go: up one dir, main page]
Skip to content

Update prediction interface to provide additional feedback to a predictor plugin #15421

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
May 20, 2021
Merged

Update prediction interface to provide additional feedback to a predictor plugin #15421

merged 8 commits into from
May 20, 2021

Conversation

daxian-dbw
Copy link
Member
@daxian-dbw daxian-dbw commented May 18, 2021 •
edited
Loading

PR Summary

Update prediction interface to provide additional feedback to a predictor plugin.
The changes are:

  1. Introduce PredictionClient to represent a client. It provides additional information about the type of the client: terminal, editor.
  2. Update ICommandPredictor interface to replace the properties SupportEarlyProcessing and AcceptFeedback with the method CanAcceptFeedback(PredictionClient client, PredictorFeedbackKind feedback). So a predictor implementation can be specific about whether it accepts each feedback.
  3. Add the feedback event OnCommandLineExecuted. It provides a predictor the information about the execution result (success or failure) of the last accepted user input.
  4. Update how delegate is created in CommandPrediction, to minimize the allocation of the delegate helper object.
  5. Add more tests and fix the fragile test.

PR Context

  1. The Az.Predictor team asked for the new feedback OnCommandExecuted, which provides the execution status (success or failure) of the last accepted user input.
  2. A predictor implementation will need the information about the client type (terminal, editor) to properly serve those 2 different scenarios, so we need to use a type to represent the client.

PR Checklist

@ghost ghost assigned anmenaga May 18, 2021
@daxian-dbw daxian-dbw requested review from anmenaga and rjmholt May 18, 2021 16:27
@daxian-dbw daxian-dbw added the CL-Experimental Indicates that a PR should be marked as an Experimental Feature in the Change Log label May 18, 2021
rjmholt
rjmholt reviewed May 18, 2021
View reviewed changes
@daxian-dbw
Copy link
Member Author

@rjmholt Thanks for your review! I've addressed your feedback, please take another look when you get a chance.

@daxian-dbw daxian-dbw mentioned this pull request May 19, 2021
Merged
22 tasks
@anmenaga anmenaga merged commit 5febcad into PowerShell:master May 20, 2021
@daxian-dbw daxian-dbw deleted the predict branch May 21, 2021 05:21
@ghost
Copy link
ghost commented May 27, 2021

🎉v7.2.0-preview.6 has been released which incorporates this pull request.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iSazonov iSazonov iSazonov left review comments

@rjmholt rjmholt rjmholt approved these changes

@anmenaga anmenaga Awaiting requested review from anmenaga

Assignees

@anmenaga anmenaga

Labels
CL-Experimental Indicates that a PR should be marked as an Experimental Feature in the Change Log
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@daxian-dbw @rjmholt @iSazonov @anmenaga
0