Fix #11068: Interactive window should allow to add commands to history without execution #11078
Conversation
…history without execution
|
Hi @AlexanderSher, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution! TTYL, DNFBOT; |
| @@ -307,6 +312,13 @@ public void ClearView() | |||
| edit.Apply(); | |||
| } | |||
|
|
|||
| if (HistoryBuffer != null) { | |||
| insertionIndex = IndexOfLastSpan(sourceSpans, ReplSpanKind.Prompt); | ||
| break; | ||
| default: | ||
| throw new ArgumentOutOfRangeException(); |
There was a problem hiding this comment.
This is an odd exception to throw here. InvalidOperationException would be more appropriate.
|
@genlu Please take a look. |
| } | ||
|
|
||
|
|
||
| var historyBuffer = _factory.CreateAndActivateBuffer(_window); |
There was a problem hiding this comment.
Looking at the implementation of the VS implementation of CreateAndActivateBuffer it seems that this is unnecessary for history buffer - it's wiring up undo manager and command filters.
Doing this should be sufficient:
IContentType contentType;
if (!window.Properties.TryGetProperty(typeof(IContentType), out contentType))
{
contentType = _contentTypeRegistry.GetContentType("text");
}
var historyBuffer = _textBufferFactoryService.CreateTextBuffer(contentType);
Also, I think we should set properties:
buffer.Properties.AddProperty(typeof(IInteractiveEvaluator), Evaluator);
buffer.Properties.AddProperty(typeof(InteractiveWindow), _window);
they are set on all language buffers.
- Move state check to the beginning of the method - Use InvalidOperationException instead of ArgumentOutOfRangeException
|
Issues fixed |
|
One more thing: could you add a comment to the public AddToHistory method that it throws InvalidOperationException when the REPL is in certain states? 👍 |
|
|
||
| break; | ||
| default: | ||
| throw new InvalidOperationException(); |
There was a problem hiding this comment.
throw ExceptionUtilities.UnexpectedValue(State); since this is supposedly to be exhaustive.
There was a problem hiding this comment.
I don't think we have ExceptionUtilities available here, or do we?
|
@MattGertz Ask mode PR to support RTVS. |
- Added test that verifies history navigation
|
👍 |
|
👍 |
No description provided.