Force refreshing suggestion in the inline view when plugin is in use (#3644)

Author: daxian-dbw

PSReadLine/Prediction.Views.cs

@@ -1082,9 +1082,29 @@ internal override void GetSuggestion(string userInput)
                 {
                     _inputText = userInput;
 
-                    if (_suggestionText == null || _suggestionText.Length <= userInput.Length ||
-                        _lastInputText.Length > userInput.Length ||
-                        !_suggestionText.StartsWith(userInput, _singleton._options.HistoryStringComparison))
+                    string currentSugText = null;
+                    bool needToRefresh = _suggestionText == null
+                        || _suggestionText.Length <= userInput.Length
+                        || _lastInputText.Length > userInput.Length
+                        || !_suggestionText.StartsWith(userInput, _singleton._options.HistoryStringComparison);
+
+
+                    // The current suggestion was from history and it still applies to the new input. However, the plugin is in use,
+                    // so we may need to force refreshing in case the plugin gives more relevant suggestion for the new input. This
+                    // is because we favor plugin over history in the inline view.
+                    if (!needToRefresh && _predictorId == Guid.Empty && UsePlugin)
+                    {
+                        // We generally want to force refreshing in this case, with only one exception -- the user accepted the next
+                        // word from the current history suggestion. That means the user is interested in the current suggestion and
+                        // thus we should keep on using.
+                        needToRefresh = !_alreadyAccepted;
+                        _alreadyAccepted = false;
+
+                        // We can reuse the current suggestion text for history to avoid an unnecessary search.
+                        currentSugText = _suggestionText;
+                    }
+
+                    if (needToRefresh)
                     {
                         _alreadyAccepted = false;
                         _suggestionText = null;
@@ -1097,7 +1117,7 @@ internal override void GetSuggestion(string userInput)
 
                         if (UseHistory)
                         {
-                            _suggestionText = GetOneHistorySuggestion(userInput);
+                            _suggestionText = currentSugText ?? GetOneHistorySuggestion(userInput);
                             _predictorId = Guid.Empty;
                             _predictorSession = null;
                         }
@@ -1215,15 +1235,14 @@ internal override void RenderSuggestion(List<StringBuilder> consoleBufferLines,
 
             internal override void OnSuggestionAccepted()
             {
-                if (!UsePlugin)
+                if (_alreadyAccepted)
                 {
                     return;
                 }
 
-                if (!_alreadyAccepted && _suggestionText != null && _predictorSession.HasValue)
+                _alreadyAccepted = true;
+                if (_suggestionText != null && _predictorSession.HasValue)
                 {
-                    _alreadyAccepted = true;
-
                     // Send feedback only if the mini-session id is specified.
                     // When it's not specified, we consider the predictor doesn't accept feedback.
                     _singleton._mockableMethods.OnSuggestionAccepted(_predictorId, _predictorSession.Value, _suggestionText);

test/InlinePredictionTest.cs

@@ -605,6 +605,48 @@ public void Inline_HistoryAndPluginSource_Acceptance()
             Assert.NotNull(_mockedMethods.commandHistory);
             Assert.Equal(1, _mockedMethods.commandHistory.Count);
             Assert.Equal("netsh show me", _mockedMethods.commandHistory[0]);
+
+            _mockedMethods.ClearPredictionFields();
+            SetHistory("netsh show me");
+            Test("netsh  SOME TEXT AFTER", Keys(
+                "netsh", CheckThat(() => AssertScreenIs(1,
+                    TokenClassification.Command, "netsh",
+                    TokenClassification.InlinePrediction, " show me")),
+                // Yeah, we still have `OnSuggestionDisplayed` fired, from the typing of each character of `nets`.
+                CheckThat(() => AssertDisplayedSuggestions(count: 1, predictorId_1, MiniSessionId, countOrIndex: -1)),
+                CheckThat(() => _mockedMethods.ClearPredictionFields()),
+
+                // Now mimic pressing a space key. This will trigger the refreshing of suggestions even though the
+                // current history suggestion still applies to the new input, because plugin is in use and we favor
+                // plugin over history results.
+                ' ', CheckThat(() => AssertScreenIs(1,
+                    TokenClassification.Command, "netsh",
+                    TokenClassification.None, " ",
+                    TokenClassification.InlinePrediction, " SOME TEXT AFTER")),
+                CheckThat(() => AssertDisplayedSuggestions(count: 1, predictorId_1, MiniSessionId, countOrIndex: -1)),
+                CheckThat(() => Assert.Equal(Guid.Empty, _mockedMethods.acceptedPredictorId)),
+                CheckThat(() => Assert.Null(_mockedMethods.acceptedSuggestion)),
+                CheckThat(() => Assert.Null(_mockedMethods.commandHistory)),
+
+                CheckThat(() => _mockedMethods.ClearPredictionFields()),
+                // 'RightArrow' will trigger 'OnSuggestionAccepted' as the suggestion is now from plugin.
+                _.RightArrow, CheckThat(() => AssertScreenIs(1,
+                    TokenClassification.Command, "netsh",
+                    TokenClassification.None, "  SOME TEXT AFTER")),
+                CheckThat(() => Assert.Empty(_mockedMethods.displayedSuggestions)),
+                CheckThat(() => Assert.Equal(predictorId_1, _mockedMethods.acceptedPredictorId)),
+                CheckThat(() => Assert.Equal("netsh  SOME TEXT AFTER", _mockedMethods.acceptedSuggestion)),
+                CheckThat(() => Assert.Null(_mockedMethods.commandHistory))
+            ));
+
+            Assert.Empty(_mockedMethods.displayedSuggestions);
+            Assert.Equal(predictorId_1, _mockedMethods.acceptedPredictorId);
+            Assert.Equal("netsh  SOME TEXT AFTER", _mockedMethods.acceptedSuggestion);
+            Assert.NotNull(_mockedMethods.commandHistory);
+            Assert.Equal(2, _mockedMethods.commandHistory.Count);
+            Assert.Equal("netsh show me", _mockedMethods.commandHistory[0]);
+            Assert.Equal("netsh  SOME TEXT AFTER", _mockedMethods.commandHistory[1]);
+            _mockedMethods.ClearPredictionFields();
         }
 
         [SkippableFact]

test/UnitTestReadLine.cs

@@ -27,7 +27,7 @@ internal class MockedMethods : IPSConsoleReadLineMockableMethods
         internal Guid acceptedPredictorId;
         internal string acceptedSuggestion;
         internal string helpContentRendered;
-        internal Dictionary<Guid, Tuple<uint, int>> displayedSuggestions = new Dictionary<Guid, Tuple<uint, int>>();
+        internal Dictionary<Guid, Tuple<uint, int>> displayedSuggestions = new();
 
         internal void ClearPredictionFields()
         {