Skip to content

Commit e182978

Browse files
kceiwkceiw
authored
return multiple suggestion (#13098)
* Return multiple suggestions. - Add a setting to configure at most how many suggestions to return. - We'll go through the command/parameters one by one and to construct the suggestion. We may return multiple suggestions for the same command but with different parameter sets; * Use camel casing in the setting fields. - Looks like other json file in $HOME/.azure use camel case or PascalCase. None use snake case. * Fix a logic error. - We'll use the command list for suggestion while the prediction list isn't available. * Stop finding suggestion when the count is met. * Improve the MockPSConsole. * Cache the last two commands from the history.
1 parent ce42552 commit e182978

16 files changed

+245
-231
lines changed

‎tools/Az.Tools.Predictor/Az.Tools.Predictor.Test/AzPredictorServiceTests.cs

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
// ----------------------------------------------------------------------------------
1414

1515
using Microsoft.Azure.PowerShell.Tools.AzPredictor.Test.Mocks;
16+
using System.Linq;
1617
using System.Management.Automation.Subsystem;
1718
using System.Threading;
1819
using Xunit;
@@ -61,12 +62,12 @@ public AzPredictorServiceTests(ModelFixture fixture)
6162
public void VerifyUsingSuggestion(string userInput)
6263
{
6364
var predictionContext = PredictionContext.Create(userInput);
64-
var expected = this._suggestionsPredictor.Query(predictionContext.InputAst, CancellationToken.None);
65-
var actual = this._service.GetSuggestion(predictionContext.InputAst, CancellationToken.None);
66-
Assert.NotNull(actual);
67-
Assert.NotNull(actual.Item1);
68-
Assert.Equal(expected, actual.Item1);
69-
Assert.Equal(PredictionSource.CurrentCommand, actual.Item2);
65+
var expected = this._suggestionsPredictor.Query(predictionContext.InputAst, 1, CancellationToken.None);
66+
var actual = this._service.GetSuggestion(predictionContext.InputAst, 1, CancellationToken.None);
67+
Assert.NotEmpty(actual);
68+
Assert.NotNull(actual.First().Item1);
69+
Assert.Equal(expected.First(), actual.First().Item1);
70+
Assert.Equal(PredictionSource.CurrentCommand, actual.First().Item2);
7071
}
7172

7273
/// <summary>
@@ -78,12 +79,12 @@ public void VerifyUsingSuggestion(string userInput)
7879
public void VerifyUsingCommand(string userInput)
7980
{
8081
var predictionContext = PredictionContext.Create(userInput);
81-
var expected = this._commandsPredictor.Query(predictionContext.InputAst, CancellationToken.None);
82-
var actual = this._service.GetSuggestion(predictionContext.InputAst, CancellationToken.None);
83-
Assert.NotNull(actual);
84-
Assert.NotNull(actual.Item1);
85-
Assert.Equal(expected, actual.Item1);
86-
Assert.Equal(PredictionSource.StaticCommands, actual.Item2);
82+
var expected = this._commandsPredictor.Query(predictionContext.InputAst, 1, CancellationToken.None);
83+
var actual = this._service.GetSuggestion(predictionContext.InputAst, 1, CancellationToken.None);
84+
Assert.NotEmpty(actual);
85+
Assert.NotNull(actual.First().Item1);
86+
Assert.Equal(expected.First(), actual.First().Item1);
87+
Assert.Equal(PredictionSource.StaticCommands, actual.First().Item2);
8788
}
8889

8990
/// <summary>
@@ -100,9 +101,8 @@ public void VerifyUsingCommand(string userInput)
100101
public void VerifyNoPrediction(string userInput)
101102
{
102103
var predictionContext = PredictionContext.Create(userInput);
103-
var actual = this._service.GetSuggestion(predictionContext.InputAst, CancellationToken.None);
104-
Assert.Null(actual.Item1);
105-
Assert.Equal(PredictionSource.None, actual.Item2);
104+
var actual = this._service.GetSuggestion(predictionContext.InputAst, 1, CancellationToken.None);
105+
Assert.Empty(actual);
106106
}
107107
}
108108
}

‎tools/Az.Tools.Predictor/Az.Tools.Predictor.Test/AzPredictorTests.cs

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,10 @@ public AzPredictorTests(ModelFixture modelFixture)
4242

4343
this._service = new MockAzPredictorService(startHistory, this._fixture.PredictionCollection[startHistory], this._fixture.CommandCollection);
4444
this._telemetryClient = new MockAzPredictorTelemetryClient();
45-
this._azPredictor = new AzPredictor(this._service, this._telemetryClient);
45+
this._azPredictor = new AzPredictor(this._service, this._telemetryClient, new Settings()
46+
{
47+
SuggestionCount = 1,
48+
});
4649
}
4750

4851
/// <summary>
@@ -138,18 +141,10 @@ public void VerifySupportedCommandMasked()
138141
public void VerifySuggestion(string userInput)
139142
{
140143
var predictionContext = PredictionContext.Create(userInput);
141-
var expected = this._service.GetSuggestion(predictionContext.InputAst, CancellationToken.None);
144+
var expected = this._service.GetSuggestion(predictionContext.InputAst, 1, CancellationToken.None);
142145
var actual = this._azPredictor.GetSuggestion(predictionContext, CancellationToken.None);
143-
if (actual == null)
144-
{
145-
Assert.Null(expected?.Item1);
146-
}
147-
else
148-
{
149-
Assert.Single(actual);
150-
Assert.Equal(expected.Item1, actual.First().SuggestionText);
151-
}
152146

147+
Assert.Equal(expected.Select(e => e.Item1), actual.Select(a => a.SuggestionText));
153148
}
154149
}
155150
}

‎tools/Az.Tools.Predictor/Az.Tools.Predictor.Test/Mocks/MockAzPredictorTelemetryClient.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public void OnSuggestionAccepted(string acceptedSuggestion)
5959
}
6060

6161
/// <inheritdoc/>
62-
public void OnGetSuggestion(string maskedUserInput, IEnumerable<Tuple<string, PredictionSource>> suggestions, bool isCancelled)
62+
public void OnGetSuggestion(string maskedUserInput, IEnumerable<ValueTuple<string, PredictionSource>> suggestions, bool isCancelled)
6363
{
6464
}
6565

‎tools/Az.Tools.Predictor/Az.Tools.Predictor.Test/PredictorTests.cs

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
// limitations under the License.
1313
// ----------------------------------------------------------------------------------
1414

15+
using System.Linq;
1516
using System.Management.Automation.Subsystem;
1617
using System.Threading;
1718
using Xunit;
@@ -50,8 +51,8 @@ public PredictorTests(ModelFixture fixture)
5051
public void GetNullPredictionWithCommandName(string userInput)
5152
{
5253
var predictionContext = PredictionContext.Create(userInput);
53-
var result = this._predictor.Query(predictionContext.InputAst, CancellationToken.None);
54-
Assert.Null(result);
54+
var result = this._predictor.Query(predictionContext.InputAst, 1, CancellationToken.None);
55+
Assert.Empty(result);
5556
}
5657

5758
/// <summary>
@@ -66,8 +67,8 @@ public void GetNullPredictionWithCommandName(string userInput)
6667
public void GetPredictionWithCommandName(string userInput)
6768
{
6869
var predictionContext = PredictionContext.Create(userInput);
69-
var result = this._predictor.Query(predictionContext.InputAst, CancellationToken.None);
70-
Assert.NotNull(result);
70+
var result = this._predictor.Query(predictionContext.InputAst, 1, CancellationToken.None);
71+
Assert.NotEmpty(result);
7172
}
7273

7374
/// <summary>
@@ -81,8 +82,8 @@ public void GetPredictionWithCommandName(string userInput)
8182
public void GetPredictionWithCommandNameParameters(string userInput)
8283
{
8384
var predictionContext = PredictionContext.Create(userInput);
84-
var result = this._predictor.Query(predictionContext.InputAst, CancellationToken.None);
85-
Assert.NotNull(result);
85+
var result = this._predictor.Query(predictionContext.InputAst, 1, CancellationToken.None);
86+
Assert.NotEmpty(result);
8687
}
8788

8889
/// <summary>
@@ -97,8 +98,8 @@ public void GetPredictionWithCommandNameParameters(string userInput)
9798
public void GetNullPredictionWithCommandNameParameters(string userInput)
9899
{
99100
var predictionContext = PredictionContext.Create(userInput);
100-
var result = this._predictor.Query(predictionContext.InputAst, CancellationToken.None);
101-
Assert.Null(result);
101+
var result = this._predictor.Query(predictionContext.InputAst, 1, CancellationToken.None);
102+
Assert.Empty(result);
102103
}
103104

104105
/// <summary>
@@ -108,9 +109,9 @@ public void GetNullPredictionWithCommandNameParameters(string userInput)
108109
public void VerifyPredictionForCommand()
109110
{
110111
var predictionContext = PredictionContext.Create("Connect-AzAccount");
111-
var result = this._predictor.Query(predictionContext.InputAst, CancellationToken.None);
112+
var result = this._predictor.Query(predictionContext.InputAst, 1, CancellationToken.None);
112113

113-
Assert.Equal("Connect-AzAccount -Credential <PSCredential> -ServicePrincipal -Tenant <>", result);
114+
Assert.Equal("Connect-AzAccount -Credential <PSCredential> -ServicePrincipal -Tenant <>", result.First());
114115
}
115116

116117
/// <summary>
@@ -120,9 +121,9 @@ public void VerifyPredictionForCommand()
120121
public void VerifyPredictionForCommandAndParameters()
121122
{
122123
var predictionContext = PredictionContext.Create("GET-AZSTORAGEACCOUNTKEY -NAME");
123-
var result = this._predictor.Query(predictionContext.InputAst, CancellationToken.None);
124+
var result = this._predictor.Query(predictionContext.InputAst, 1, CancellationToken.None);
124125

125-
Assert.Equal("Get-AzStorageAccountKey -Name 'ContosoStorage' -ResourceGroupName 'ContosoGroup02'", result);
126+
Assert.Equal("Get-AzStorageAccountKey -Name 'ContosoStorage' -ResourceGroupName 'ContosoGroup02'", result.First());
126127
}
127128
}
128129
}

‎tools/Az.Tools.Predictor/Az.Tools.Predictor/AzPredictor.cs

Lines changed: 69 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ namespace Microsoft.Azure.PowerShell.Tools.AzPredictor
2929
/// <summary>
3030
/// The implementation of a <see cref="ICommandPredictor"/> to provide suggestion in PSReadLine.
3131
/// </summary>
32-
public sealed class AzPredictor : ICommandPredictor
32+
internal sealed class AzPredictor : ICommandPredictor
3333
{
3434
/// <inhericdoc />
3535
public string Name => "Az Predictor";
@@ -44,7 +44,7 @@ public sealed class AzPredictor : ICommandPredictor
4444
public bool SupportEarlyProcessing => true;
4545

4646
/// <inhericdoc />
47-
public bool AcceptFeedback => false;
47+
public bool AcceptFeedback => true;
4848

4949
internal static readonly Guid Identifier = new Guid("599d1760-4ee1-4ed2-806e-f2a1b1a0ba4d");
5050

@@ -56,52 +56,85 @@ public sealed class AzPredictor : ICommandPredictor
5656

5757
private readonly IAzPredictorService _service;
5858
private readonly ITelemetryClient _telemetryClient;
59+
private readonly Settings _settings;
60+
61+
private Queue<string> _lastTwoMaskedCommands = new Queue<string>(AzPredictorConstants.CommandHistoryCountToProcess);
5962

6063
/// <summary>
6164
/// Constructs a new instance of <see cref="AzPredictor"/>
6265
/// </summary>
6366
/// <param name="service">The service that provides the suggestion</param>
6467
/// <param name="telemetryClient">The client to collect telemetry</param>
65-
public AzPredictor(IAzPredictorService service, ITelemetryClient telemetryClient)
68+
/// <param name="settings">The settings of the service</param>
69+
public AzPredictor(IAzPredictorService service, ITelemetryClient telemetryClient, Settings settings)
6670
{
6771
this._service = service;
6872
this._telemetryClient = telemetryClient;
73+
this._settings = settings;
6974
}
7075

7176
/// <inhericdoc />
7277
public void StartEarlyProcessing(IReadOnlyList<string> history)
7378
{
7479
if (history.Count > 0)
7580
{
76-
var historyLines = history.TakeLast(AzPredictorConstants.CommandHistoryCountToProcess);
81+
if (_lastTwoMaskedCommands.Any())
82+
{
83+
_lastTwoMaskedCommands.Dequeue();
84+
}
85+
else
86+
{
87+
// This is the first time we populate our record. Push the second to last command in history to the
88+
// queue. If there is only one command in history, push the command placeholder.
89+
90+
if (history.Count() > 1)
91+
{
92+
string secondToLastLine = history.TakeLast(AzPredictorConstants.CommandHistoryCountToProcess).First();
93+
var secondToLastCommand = GetAstAndMaskedCommandLine(secondToLastLine);
94+
_lastTwoMaskedCommands.Enqueue(secondToLastCommand.Item2);
95+
_service.RecordHistory(secondToLastCommand.Item1);
96+
}
97+
else
98+
{
99+
_lastTwoMaskedCommands.Enqueue(AzPredictorConstants.CommandPlaceholder);
100+
// We only extract parameter values from the command line in _service.RecordHistory.
101+
// So we don't need to do that for a placeholder.
102+
}
103+
}
104+
105+
string lastLine = history.Last();
106+
var lastCommand = GetAstAndMaskedCommandLine(lastLine);
77107

78-
while (historyLines.Count() < AzPredictorConstants.CommandHistoryCountToProcess)
108+
_lastTwoMaskedCommands.Enqueue(lastCommand.Item2);
109+
110+
if ((lastCommand.Item2 != null) && !string.Equals(AzPredictorConstants.CommandPlaceholder, lastCommand.Item2, StringComparison.Ordinal))
79111
{
80-
historyLines = historyLines.Prepend(AzPredictorConstants.CommandPlaceholder);
112+
_service.RecordHistory(lastCommand.Item1);
81113
}
82114

83-
var commandAsts = historyLines.Select((h) =>
84-
{
85-
var ast = Parser.ParseInput(h, out Token[] tokens, out _);
86-
var allAsts = ast?.FindAll((ast) => ast is CommandAst, true);
87-
return allAsts?.LastOrDefault() as CommandAst;
88-
}).ToArray();
115+
_telemetryClient.OnHistory(lastCommand.Item2);
116+
_service.RequestPredictions(_lastTwoMaskedCommands);
117+
}
89118

90-
var maskedHistoryLines = commandAsts.Select((c) =>
91-
{
92-
var commandName = c?.CommandElements?.FirstOrDefault().ToString();
119+
ValueTuple<CommandAst, string> GetAstAndMaskedCommandLine(string commandLine)
120+
{
121+
var asts = Parser.ParseInput(commandLine, out _, out _);
122+
var allNestedAsts = asts?.FindAll((ast) => ast is CommandAst, true);
123+
var commandAst = allNestedAsts?.LastOrDefault() as CommandAst;
124+
string maskedCommandLine = null;
93125

94-
if (!_service.IsSupportedCommand(commandName))
95-
{
96-
return AzPredictorConstants.CommandPlaceholder;
97-
}
126+
var commandName = commandAst?.CommandElements?.FirstOrDefault().ToString();
98127

99-
return AzPredictor.MaskCommandLine(c);
100-
});
128+
if (_service.IsSupportedCommand(commandName))
129+
{
130+
maskedCommandLine = AzPredictor.MaskCommandLine(commandAst);
131+
}
132+
else
133+
{
134+
maskedCommandLine = AzPredictorConstants.CommandPlaceholder;
135+
}
101136

102-
_telemetryClient.OnHistory(maskedHistoryLines.Last());
103-
_service.RecordHistory(commandAsts);
104-
_service.RequestPredictions(maskedHistoryLines);
137+
return ValueTuple.Create(commandAst, maskedCommandLine);
105138
}
106139
}
107140

@@ -121,19 +154,19 @@ public List<PredictiveSuggestion> GetSuggestion(PredictionContext context, Cance
121154
// is prefixed with `userInput`, it should be ordered before result that is not prefixed
122155
// with `userInput`.
123156

124-
Tuple<string, PredictionSource> result = Tuple.Create<string, PredictionSource>(null, PredictionSource.None);
157+
IEnumerable<ValueTuple<string, PredictionSource>> suggestions = Enumerable.Empty<ValueTuple<string, PredictionSource>>();
125158

126159
try
127160
{
128-
result = _service.GetSuggestion(context.InputAst, cancellationToken);
161+
suggestions = _service.GetSuggestion(context.InputAst, _settings.SuggestionCount.Value, cancellationToken);
129162

130-
if (result?.Item1 != null)
131-
{
132-
cancellationToken.ThrowIfCancellationRequested();
133-
var userInput = context.InputAst.Extent.Text;
134-
var fullSuggestion = MergeStrings(userInput, result.Item1);
135-
return new List<PredictiveSuggestion>() { new PredictiveSuggestion(fullSuggestion) };
136-
}
163+
cancellationToken.ThrowIfCancellationRequested();
164+
var userInput = context.InputAst.Extent.Text;
165+
return suggestions.Select((r, index) =>
166+
{
167+
return new PredictiveSuggestion(MergeStrings(userInput, r.Item1));
168+
})
169+
.ToList();
137170
}
138171
catch (Exception e) when (!(e is OperationCanceledException))
139172
{
@@ -142,11 +175,10 @@ public List<PredictiveSuggestion> GetSuggestion(PredictionContext context, Cance
142175
finally
143176
{
144177
var maskedCommandLine = MaskCommandLine(context.InputAst.FindAll((ast) => ast is CommandAst, true).LastOrDefault() as CommandAst);
145-
_telemetryClient.OnGetSuggestion(maskedCommandLine, new Tuple<string, PredictionSource>[] { result },
146-
cancellationToken.IsCancellationRequested);
178+
_telemetryClient.OnGetSuggestion(maskedCommandLine, suggestions, cancellationToken.IsCancellationRequested);
147179
}
148180

149-
return null;
181+
return new List<PredictiveSuggestion>();
150182
}
151183

152184
// Merge strings a and b such that the prefix of b is deleted if it is the suffix of a
@@ -232,7 +264,7 @@ public void OnImport()
232264
var settings = Settings.GetSettings();
233265
var telemetryClient = new AzPredictorTelemetryClient();
234266
var azPredictorService = new AzPredictorService(settings.ServiceUri, telemetryClient);
235-
var predictor = new AzPredictor(azPredictorService, telemetryClient);
267+
var predictor = new AzPredictor(azPredictorService, telemetryClient, settings);
236268
SubsystemManager.RegisterSubsystem<ICommandPredictor, AzPredictor>(predictor);
237269
}
238270
}

0 commit comments

Comments
 (0)