Skip to content

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

Merged
anmenaga merged 8 commits into
PowerShell:masterfrom
daxian-dbw:predict
May 20, 2021
Merged

Update prediction interface to provide additional feedback to a predictor plugin#15421
anmenaga merged 8 commits into
PowerShell:masterfrom
daxian-dbw:predict

Conversation

@daxian-dbw
Copy link
Copy Markdown
Member

@daxian-dbw daxian-dbw commented May 18, 2021

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
Comment thread test/xUnit/csharp/test_Prediction.cs Outdated
@daxian-dbw
Copy link
Copy Markdown
Member Author

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

@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
Copy Markdown

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

Labels

CL-Experimental Indicates that a PR should be marked as an Experimental Feature in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants