React to API feedback on VisualStudioRazorParser.

- Changed `ReparseAsync` to be `QueueReparse`. It's now async void to not give the misconception that it blocks until a reparse has been completed.
- Removed `IContextChangedListener`. People can get the same effect of the interface by retrieving the document tracker interface via the `RazorEditorFactoryService` and then when its context changes getting the parser.
- Exposed `TryGetParser` to aid in replacing `IContextChangedListener`.
- Updated tests to not rely on `IContextChangedListener`.
This commit is contained in:
N. Taylor Mullen 2017-10-24 17:11:38 -07:00
parent d027697389
commit ab62ea9321
8 changed files with 17 additions and 99 deletions

View File

@ -30,7 +30,6 @@ namespace Microsoft.VisualStudio.Editor.Razor
private readonly object IdleLock = new object(); private readonly object IdleLock = new object();
private readonly ICompletionBroker _completionBroker; private readonly ICompletionBroker _completionBroker;
private readonly IEnumerable<IContextChangedListener> _contextChangedListeners;
private readonly VisualStudioDocumentTracker _documentTracker; private readonly VisualStudioDocumentTracker _documentTracker;
private readonly ForegroundDispatcher _dispatcher; private readonly ForegroundDispatcher _dispatcher;
private readonly RazorTemplateEngineFactoryService _templateEngineFactory; private readonly RazorTemplateEngineFactoryService _templateEngineFactory;
@ -51,8 +50,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
VisualStudioDocumentTracker documentTracker, VisualStudioDocumentTracker documentTracker,
RazorTemplateEngineFactoryService templateEngineFactory, RazorTemplateEngineFactoryService templateEngineFactory,
ErrorReporter errorReporter, ErrorReporter errorReporter,
ICompletionBroker completionBroker, ICompletionBroker completionBroker)
IEnumerable<IContextChangedListener> contextChangedListeners)
{ {
if (dispatcher == null) if (dispatcher == null)
{ {
@ -79,16 +77,10 @@ namespace Microsoft.VisualStudio.Editor.Razor
throw new ArgumentNullException(nameof(completionBroker)); throw new ArgumentNullException(nameof(completionBroker));
} }
if (contextChangedListeners == null)
{
throw new ArgumentNullException(nameof(contextChangedListeners));
}
_dispatcher = dispatcher; _dispatcher = dispatcher;
_templateEngineFactory = templateEngineFactory; _templateEngineFactory = templateEngineFactory;
_errorReporter = errorReporter; _errorReporter = errorReporter;
_completionBroker = completionBroker; _completionBroker = completionBroker;
_contextChangedListeners = contextChangedListeners;
_documentTracker = documentTracker; _documentTracker = documentTracker;
_documentTracker.ContextChanged += DocumentTracker_ContextChanged; _documentTracker.ContextChanged += DocumentTracker_ContextChanged;
@ -110,7 +102,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
// Used in unit tests to ensure we can block background idle work. // Used in unit tests to ensure we can block background idle work.
internal ManualResetEventSlim BlockBackgroundIdleWork { get; set; } internal ManualResetEventSlim BlockBackgroundIdleWork { get; set; }
public override async Task ReparseAsync() public override void QueueReparse()
{ {
// Can be called from any thread // Can be called from any thread
@ -120,7 +112,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
} }
else else
{ {
await Task.Factory.StartNew(ReparseOnForeground, null, CancellationToken.None, TaskCreationOptions.None, _dispatcher.ForegroundScheduler); Task.Factory.StartNew(ReparseOnForeground, null, CancellationToken.None, TaskCreationOptions.None, _dispatcher.ForegroundScheduler);
} }
} }
@ -145,11 +137,9 @@ namespace Microsoft.VisualStudio.Editor.Razor
return; return;
} }
NotifyParserContextChanged();
// We have a new parser, force a reparse to generate new document information. Note that this // We have a new parser, force a reparse to generate new document information. Note that this
// only blocks until the reparse change has been queued. // only blocks until the reparse change has been queued.
ReparseAsync().GetAwaiter().GetResult(); QueueReparse();
} }
// Internal for testing // Internal for testing
@ -171,21 +161,6 @@ namespace Microsoft.VisualStudio.Editor.Razor
return true; return true;
} }
// Internal for testing
internal void NotifyParserContextChanged()
{
_dispatcher.AssertForegroundThread();
// This is temporary until we own the TagHelper resolution system. At that point the parser will push out updates
// via DocumentStructureChangedEvents when contexts change. For now, listeners need to know more information about
// the parser. In the case that the tracker does not belong to a supported project the editor will tear down its
// attachment to the parser when it recognizes the document closing.
foreach (var contextChangeListener in _contextChangedListeners)
{
contextChangeListener.OnContextChanged(this);
}
}
// Internal for testing // Internal for testing
internal void StartParser() internal void StartParser()
{ {
@ -303,8 +278,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
} }
} }
// This only blocks until the reparse change has been queued. QueueReparse();
ReparseAsync().GetAwaiter().GetResult();
} }
private void ReparseOnForeground(object state) private void ReparseOnForeground(object state)

View File

@ -1,13 +0,0 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
namespace Microsoft.VisualStudio.Editor.Razor
{
/// <summary>
/// This class will cease to be useful once the Razor tooling owns TagHelper discovery
/// </summary>
public interface IContextChangedListener
{
void OnContextChanged(VisualStudioRazorParser parser);
}
}

View File

@ -9,7 +9,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
{ {
public abstract bool TryGetDocumentTracker(ITextBuffer textBuffer, out VisualStudioDocumentTracker documentTracker); public abstract bool TryGetDocumentTracker(ITextBuffer textBuffer, out VisualStudioDocumentTracker documentTracker);
internal abstract bool TryGetParser(ITextBuffer textBuffer, out VisualStudioRazorParser parser); public abstract bool TryGetParser(ITextBuffer textBuffer, out VisualStudioRazorParser parser);
internal abstract bool TryGetSmartIndenter(ITextBuffer textBuffer, out BraceSmartIndenter braceSmartIndenter); internal abstract bool TryGetSmartIndenter(ITextBuffer textBuffer, out BraceSmartIndenter braceSmartIndenter);
} }

View File

@ -2,7 +2,6 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using System; using System;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Razor.Language; using Microsoft.AspNetCore.Razor.Language;
using Microsoft.VisualStudio.Text; using Microsoft.VisualStudio.Text;
@ -22,6 +21,6 @@ namespace Microsoft.VisualStudio.Editor.Razor
public abstract ITextBuffer TextBuffer { get; } public abstract ITextBuffer TextBuffer { get; }
public abstract Task ReparseAsync(); public abstract void QueueReparse();
} }
} }

View File

@ -69,7 +69,7 @@ namespace Microsoft.VisualStudio.LanguageServices.Razor.Editor
return true; return true;
} }
internal override bool TryGetParser(ITextBuffer textBuffer, out VisualStudioRazorParser parser) public override bool TryGetParser(ITextBuffer textBuffer, out VisualStudioRazorParser parser)
{ {
if (textBuffer == null) if (textBuffer == null)
{ {

View File

@ -19,13 +19,11 @@ namespace Microsoft.VisualStudio.LanguageServices.Razor.Editor
private readonly ForegroundDispatcher _dispatcher; private readonly ForegroundDispatcher _dispatcher;
private readonly TemplateEngineFactoryService _templateEngineFactoryService; private readonly TemplateEngineFactoryService _templateEngineFactoryService;
private readonly ICompletionBroker _completionBroker; private readonly ICompletionBroker _completionBroker;
private readonly IEnumerable<IContextChangedListener> _parserContextChangedListeners;
private readonly ErrorReporter _errorReporter; private readonly ErrorReporter _errorReporter;
[ImportingConstructor] [ImportingConstructor]
public DefaultVisualStudioRazorParserFactory( public DefaultVisualStudioRazorParserFactory(
ICompletionBroker completionBroker, ICompletionBroker completionBroker,
[ImportMany(typeof(IContextChangedListener))] IEnumerable<IContextChangedListener> parserContextChangedListeners,
[Import(typeof(VisualStudioWorkspace))] Workspace workspace) [Import(typeof(VisualStudioWorkspace))] Workspace workspace)
{ {
if (completionBroker == null) if (completionBroker == null)
@ -33,18 +31,12 @@ namespace Microsoft.VisualStudio.LanguageServices.Razor.Editor
throw new ArgumentNullException(nameof(completionBroker)); throw new ArgumentNullException(nameof(completionBroker));
} }
if (parserContextChangedListeners == null)
{
throw new ArgumentNullException(nameof(parserContextChangedListeners));
}
if (workspace == null) if (workspace == null)
{ {
throw new ArgumentNullException(nameof(workspace)); throw new ArgumentNullException(nameof(workspace));
} }
_completionBroker = completionBroker; _completionBroker = completionBroker;
_parserContextChangedListeners = parserContextChangedListeners;
_dispatcher = workspace.Services.GetRequiredService<ForegroundDispatcher>(); _dispatcher = workspace.Services.GetRequiredService<ForegroundDispatcher>();
_errorReporter = workspace.Services.GetRequiredService<ErrorReporter>(); _errorReporter = workspace.Services.GetRequiredService<ErrorReporter>();
var razorLanguageServices = workspace.Services.GetLanguageServices(RazorLanguage.Name); var razorLanguageServices = workspace.Services.GetLanguageServices(RazorLanguage.Name);
@ -65,8 +57,7 @@ namespace Microsoft.VisualStudio.LanguageServices.Razor.Editor
documentTracker, documentTracker,
_templateEngineFactoryService, _templateEngineFactoryService,
_errorReporter, _errorReporter,
_completionBroker, _completionBroker);
_parserContextChangedListeners);
return parser; return parser;
} }
} }

View File

@ -38,8 +38,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
documentTracker, documentTracker,
CreateTemplateEngineFactory(), CreateTemplateEngineFactory(),
new DefaultErrorReporter(), new DefaultErrorReporter(),
new TestCompletionBroker(), new TestCompletionBroker()))
Enumerable.Empty<IContextChangedListener>()))
{ {
parser.DocumentTracker_ContextChanged(null, null); parser.DocumentTracker_ContextChanged(null, null);
var changed = new StringTextSnapshot("Foo @bap Daz"); var changed = new StringTextSnapshot("Foo @bap Daz");
@ -528,8 +527,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
documentTracker, documentTracker,
templateEngineFactory, templateEngineFactory,
new DefaultErrorReporter(), new DefaultErrorReporter(),
new TestCompletionBroker(), new TestCompletionBroker())
Enumerable.Empty<IContextChangedListener>())
{ {
// We block idle work with the below reset events. Therefore, make tests fast and have the idle timer fire as soon as possible. // We block idle work with the below reset events. Therefore, make tests fast and have the idle timer fire as soon as possible.
IdleDelay = TimeSpan.FromMilliseconds(1), IdleDelay = TimeSpan.FromMilliseconds(1),

View File

@ -35,8 +35,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
CreateDocumentTracker(), CreateDocumentTracker(),
Mock.Of<RazorTemplateEngineFactoryService>(), Mock.Of<RazorTemplateEngineFactoryService>(),
new DefaultErrorReporter(), new DefaultErrorReporter(),
Mock.Of<ICompletionBroker>(), Mock.Of<ICompletionBroker>())
Enumerable.Empty<IContextChangedListener>())
{ {
BlockBackgroundIdleWork = new ManualResetEventSlim(), BlockBackgroundIdleWork = new ManualResetEventSlim(),
IdleDelay = TimeSpan.FromSeconds(5) IdleDelay = TimeSpan.FromSeconds(5)
@ -66,8 +65,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
CreateDocumentTracker(), CreateDocumentTracker(),
Mock.Of<RazorTemplateEngineFactoryService>(), Mock.Of<RazorTemplateEngineFactoryService>(),
new DefaultErrorReporter(), new DefaultErrorReporter(),
Mock.Of<ICompletionBroker>(), Mock.Of<ICompletionBroker>())
Enumerable.Empty<IContextChangedListener>())
{ {
BlockBackgroundIdleWork = new ManualResetEventSlim(), BlockBackgroundIdleWork = new ManualResetEventSlim(),
IdleDelay = TimeSpan.FromSeconds(5) IdleDelay = TimeSpan.FromSeconds(5)
@ -96,8 +94,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
CreateDocumentTracker(), CreateDocumentTracker(),
Mock.Of<RazorTemplateEngineFactoryService>(), Mock.Of<RazorTemplateEngineFactoryService>(),
new DefaultErrorReporter(), new DefaultErrorReporter(),
Mock.Of<ICompletionBroker>(), Mock.Of<ICompletionBroker>()))
Enumerable.Empty<IContextChangedListener>()))
{ {
parser.StartParser(); parser.StartParser();
@ -121,8 +118,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
documentTracker, documentTracker,
Mock.Of<RazorTemplateEngineFactoryService>(), Mock.Of<RazorTemplateEngineFactoryService>(),
new DefaultErrorReporter(), new DefaultErrorReporter(),
Mock.Of<ICompletionBroker>(), Mock.Of<ICompletionBroker>()))
Enumerable.Empty<IContextChangedListener>()))
{ {
// Act // Act
parser.StartParser(); parser.StartParser();
@ -133,31 +129,6 @@ namespace Microsoft.VisualStudio.Editor.Razor
} }
} }
[ForegroundFact]
public void NotifyParserContextChanged_NotifiesListeners()
{
// Arrange
var listener1 = new Mock<IContextChangedListener>();
listener1.Setup(l => l.OnContextChanged(It.IsAny<VisualStudioRazorParser>()));
var listener2 = new Mock<IContextChangedListener>();
listener2.Setup(l => l.OnContextChanged(It.IsAny<VisualStudioRazorParser>()));
using (var parser = new DefaultVisualStudioRazorParser(
Dispatcher,
CreateDocumentTracker(),
Mock.Of<RazorTemplateEngineFactoryService>(),
new DefaultErrorReporter(),
Mock.Of<ICompletionBroker>(),
new[] { listener1.Object, listener2.Object }))
{
// Act
parser.NotifyParserContextChanged();
// Assert
listener1.Verify();
listener2.Verify();
}
}
[ForegroundFact] [ForegroundFact]
public void TryReinitializeParser_ReturnsTrue_IfProjectIsSupported() public void TryReinitializeParser_ReturnsTrue_IfProjectIsSupported()
{ {
@ -167,8 +138,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
CreateDocumentTracker(isSupportedProject: true), CreateDocumentTracker(isSupportedProject: true),
Mock.Of<RazorTemplateEngineFactoryService>(), Mock.Of<RazorTemplateEngineFactoryService>(),
new DefaultErrorReporter(), new DefaultErrorReporter(),
Mock.Of<ICompletionBroker>(), Mock.Of<ICompletionBroker>()))
Enumerable.Empty<IContextChangedListener>()))
{ {
// Act // Act
var result = parser.TryReinitializeParser(); var result = parser.TryReinitializeParser();
@ -187,8 +157,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
CreateDocumentTracker(isSupportedProject: false), CreateDocumentTracker(isSupportedProject: false),
Mock.Of<RazorTemplateEngineFactoryService>(), Mock.Of<RazorTemplateEngineFactoryService>(),
new DefaultErrorReporter(), new DefaultErrorReporter(),
Mock.Of<ICompletionBroker>(), Mock.Of<ICompletionBroker>()))
Enumerable.Empty<IContextChangedListener>()))
{ {
// Act // Act
var result = parser.TryReinitializeParser(); var result = parser.TryReinitializeParser();