From d0a05bf958e796b277d58297cbf36989e328ad8c Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 14 Jan 2025 12:59:59 -0800 Subject: [PATCH 1/3] Hook brace-matching up to the 'responsive completion' editor option --- .../CSharpBraceCompletionServiceFactory.cs | 8 +-- ...nSessionProvider.BraceCompletionSession.cs | 60 ++++++++++++------- .../BraceCompletionSessionProvider.cs | 10 +++- ...essAndGreaterThanBraceCompletionService.cs | 21 +++---- .../AbstractBraceCompletionService.cs | 1 + 5 files changed, 58 insertions(+), 42 deletions(-) diff --git a/src/EditorFeatures/CSharp/AutomaticCompletion/CSharpBraceCompletionServiceFactory.cs b/src/EditorFeatures/CSharp/AutomaticCompletion/CSharpBraceCompletionServiceFactory.cs index b7bf2b80428c7..1eecd0bfdb27a 100644 --- a/src/EditorFeatures/CSharp/AutomaticCompletion/CSharpBraceCompletionServiceFactory.cs +++ b/src/EditorFeatures/CSharp/AutomaticCompletion/CSharpBraceCompletionServiceFactory.cs @@ -5,7 +5,6 @@ using System; using System.Collections.Generic; using System.Composition; -using System.Linq; using Microsoft.CodeAnalysis.AutomaticCompletion; using Microsoft.CodeAnalysis.BraceCompletion; using Microsoft.CodeAnalysis.Host.Mef; @@ -15,7 +14,6 @@ namespace Microsoft.CodeAnalysis.Editor.CSharp.AutomaticCompletion; [ExportLanguageService(typeof(IBraceCompletionServiceFactory), LanguageNames.CSharp), Shared] [method: ImportingConstructor] [method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] -internal class CSharpBraceCompletionServiceFactory( - [ImportMany] IEnumerable> braceCompletionServices) : AbstractBraceCompletionServiceFactory(braceCompletionServices, LanguageNames.CSharp) -{ -} +internal sealed class CSharpBraceCompletionServiceFactory( + [ImportMany] IEnumerable> braceCompletionServices) + : AbstractBraceCompletionServiceFactory(braceCompletionServices, LanguageNames.CSharp); diff --git a/src/EditorFeatures/Core/AutomaticCompletion/BraceCompletionSessionProvider.BraceCompletionSession.cs b/src/EditorFeatures/Core/AutomaticCompletion/BraceCompletionSessionProvider.BraceCompletionSession.cs index ae11d57312352..2bf95ff15ea25 100644 --- a/src/EditorFeatures/Core/AutomaticCompletion/BraceCompletionSessionProvider.BraceCompletionSession.cs +++ b/src/EditorFeatures/Core/AutomaticCompletion/BraceCompletionSessionProvider.BraceCompletionSession.cs @@ -33,22 +33,25 @@ internal partial class BraceCompletionSessionProvider // want to re-implement logics base session provider already provides. so I ported editor's default session and // modified it little bit so that we can use it as base class. private class BraceCompletionSession( + BraceCompletionSessionProvider provider, ITextView textView, ITextBuffer subjectBuffer, SnapshotPoint openingPoint, char openingBrace, char closingBrace, ITextUndoHistory undoHistory, - IEditorOperationsFactoryService editorOperationsFactoryService, - EditorOptionsService editorOptionsService, IBraceCompletionService service, - IThreadingContext threadingContext) : IBraceCompletionSession + int timeoutThreshold) : IBraceCompletionSession { - private readonly ITextUndoHistory _undoHistory = undoHistory; - private readonly IEditorOperations _editorOperations = editorOperationsFactoryService.GetEditorOperations(textView); - private readonly EditorOptionsService _editorOptionsService = editorOptionsService; + private readonly BraceCompletionSessionProvider _provider = provider; + + private readonly IEditorOperations _editorOperations = provider._editorOperationsFactoryService.GetEditorOperations(textView); private readonly IBraceCompletionService _service = service; - private readonly IThreadingContext _threadingContext = threadingContext; + private readonly ITextUndoHistory _undoHistory = undoHistory; + private readonly int _timeoutThreshold = timeoutThreshold; + + private IThreadingContext ThreadingContext => _provider._threadingContext; + private EditorOptionsService EditorOptionsService => _provider._editorOptionsService; public char OpeningBrace { get; } = openingBrace; public char ClosingBrace { get; } = closingBrace; @@ -63,17 +66,30 @@ private class BraceCompletionSession( public void Start() { - _threadingContext.ThrowIfNotOnUIThread(); + ThreadingContext.ThrowIfNotOnUIThread(); - // Brace completion is not cancellable. - var success = _threadingContext.JoinableTaskFactory.Run(() => TryStartAsync(CancellationToken.None)); - if (!success) + var cancellationTokenSource = new CancellationTokenSource(); + var cancellationToken = cancellationTokenSource.Token; + + if (_timeoutThreshold > 0) + cancellationTokenSource.CancelAfter(_timeoutThreshold); + + try + { + var success = ThreadingContext.JoinableTaskFactory.Run(() => TryStartAsync(cancellationToken)); + if (!success) + EndSession(); + } + catch (OperationCanceledException) + { EndSession(); + } } private async Task TryStartAsync(CancellationToken cancellationToken) { - _threadingContext.ThrowIfNotOnUIThread(); + ThreadingContext.ThrowIfNotOnUIThread(); + cancellationToken.ThrowIfCancellationRequested(); var closingSnapshotPoint = ClosingPoint.GetPoint(SubjectBuffer.CurrentSnapshot); if (closingSnapshotPoint.Position < 1) @@ -118,7 +134,7 @@ private async Task TryStartAsync(CancellationToken cancellationToken) if (TryGetBraceCompletionContext(out var contextAfterStart, cancellationToken)) { - var indentationOptions = SubjectBuffer.GetIndentationOptions(_editorOptionsService, document.Project.GetFallbackAnalyzerOptions(), contextAfterStart.Document.LanguageServices, explicitFormat: false); + var indentationOptions = SubjectBuffer.GetIndentationOptions(EditorOptionsService, document.Project.GetFallbackAnalyzerOptions(), contextAfterStart.Document.LanguageServices, explicitFormat: false); var changesAfterStart = _service.GetTextChangesAfterCompletion(contextAfterStart, indentationOptions, cancellationToken); if (changesAfterStart != null) { @@ -132,7 +148,7 @@ private async Task TryStartAsync(CancellationToken cancellationToken) public void PreBackspace(out bool handledCommand) { - _threadingContext.ThrowIfNotOnUIThread(); + ThreadingContext.ThrowIfNotOnUIThread(); handledCommand = false; var caretPos = this.GetCaretPosition(); @@ -172,7 +188,7 @@ public void PostBackspace() public void PreOverType(out bool handledCommand) { - _threadingContext.ThrowIfNotOnUIThread(); + ThreadingContext.ThrowIfNotOnUIThread(); handledCommand = false; if (ClosingPoint == null) { @@ -240,7 +256,7 @@ public void PostOverType() public void PreTab(out bool handledCommand) { - _threadingContext.ThrowIfNotOnUIThread(); + ThreadingContext.ThrowIfNotOnUIThread(); handledCommand = false; if (!HasForwardTyping) @@ -264,7 +280,7 @@ public void PreReturn(out bool handledCommand) public void PostReturn() { - _threadingContext.ThrowIfNotOnUIThread(); + ThreadingContext.ThrowIfNotOnUIThread(); if (this.GetCaretPosition().HasValue) { var closingSnapshotPoint = ClosingPoint.GetPoint(SubjectBuffer.CurrentSnapshot); @@ -276,7 +292,7 @@ public void PostReturn() return; } - var indentationOptions = SubjectBuffer.GetIndentationOptions(_editorOptionsService, context.FallbackOptions, context.Document.LanguageServices, explicitFormat: false); + var indentationOptions = SubjectBuffer.GetIndentationOptions(EditorOptionsService, context.FallbackOptions, context.Document.LanguageServices, explicitFormat: false); var changesAfterReturn = _service.GetTextChangeAfterReturn(context, indentationOptions, CancellationToken.None); if (changesAfterReturn != null) { @@ -321,7 +337,7 @@ private bool HasForwardTyping { get { - _threadingContext.ThrowIfNotOnUIThread(); + ThreadingContext.ThrowIfNotOnUIThread(); var closingSnapshotPoint = ClosingPoint.GetPoint(SubjectBuffer.CurrentSnapshot); if (closingSnapshotPoint.Position > 0) @@ -366,7 +382,7 @@ internal ITextUndoTransaction CreateUndoTransaction() private void MoveCaretToClosingPoint() { - _threadingContext.ThrowIfNotOnUIThread(); + ThreadingContext.ThrowIfNotOnUIThread(); var closingSnapshotPoint = ClosingPoint.GetPoint(SubjectBuffer.CurrentSnapshot); // find the position just after the closing brace in the view's text buffer @@ -396,7 +412,7 @@ private bool TryGetBraceCompletionContext(out BraceCompletionContext context, Ca private BraceCompletionContext GetBraceCompletionContext(ParsedDocument document, StructuredAnalyzerConfigOptions fallbackOptions) { - _threadingContext.ThrowIfNotOnUIThread(); + ThreadingContext.ThrowIfNotOnUIThread(); var snapshot = SubjectBuffer.CurrentSnapshot; var closingSnapshotPoint = ClosingPoint.GetPosition(snapshot); @@ -409,7 +425,7 @@ private BraceCompletionContext GetBraceCompletionContext(ParsedDocument document private void ApplyBraceCompletionResult(BraceCompletionResult result) { - _threadingContext.ThrowIfNotOnUIThread(); + ThreadingContext.ThrowIfNotOnUIThread(); using var edit = SubjectBuffer.CreateEdit(); foreach (var change in result.TextChanges) { diff --git a/src/EditorFeatures/Core/AutomaticCompletion/BraceCompletionSessionProvider.cs b/src/EditorFeatures/Core/AutomaticCompletion/BraceCompletionSessionProvider.cs index 6d330dd5daf7b..9dccc2c4981c0 100644 --- a/src/EditorFeatures/Core/AutomaticCompletion/BraceCompletionSessionProvider.cs +++ b/src/EditorFeatures/Core/AutomaticCompletion/BraceCompletionSessionProvider.cs @@ -47,6 +47,10 @@ internal partial class BraceCompletionSessionProvider( public bool TryCreateSession(ITextView textView, SnapshotPoint openingPoint, char openingBrace, char closingBrace, out IBraceCompletionSession session) { _threadingContext.ThrowIfNotOnUIThread(); + + var isResponsive = textView.Options.GetOptionValue(DefaultOptions.ResponsiveCompletionOptionId); + var responsiveThreshold = isResponsive ? textView.Options.GetOptionValue(DefaultOptions.ResponsiveCompletionThresholdOptionId) : -1; + var textSnapshot = openingPoint.Snapshot; var document = textSnapshot.GetOpenDocumentInCurrentContextWithChanges(); if (document != null) @@ -63,9 +67,9 @@ public bool TryCreateSession(ITextView textView, SnapshotPoint openingPoint, cha { var undoHistory = _undoManager.GetTextBufferUndoManager(textView.TextBuffer).TextBufferUndoHistory; session = new BraceCompletionSession( - textView, openingPoint.Snapshot.TextBuffer, openingPoint, openingBrace, closingBrace, - undoHistory, _editorOperationsFactoryService, _editorOptionsService, - editorSession, _threadingContext); + this, textView, openingPoint.Snapshot.TextBuffer, + openingPoint, openingBrace, closingBrace, + undoHistory, editorSession, responsiveThreshold); return true; } } diff --git a/src/Features/CSharp/Portable/BraceCompletion/LessAndGreaterThanBraceCompletionService.cs b/src/Features/CSharp/Portable/BraceCompletion/LessAndGreaterThanBraceCompletionService.cs index 6bb2c97fbb61f..2408c9359d9c6 100644 --- a/src/Features/CSharp/Portable/BraceCompletion/LessAndGreaterThanBraceCompletionService.cs +++ b/src/Features/CSharp/Portable/BraceCompletion/LessAndGreaterThanBraceCompletionService.cs @@ -35,20 +35,22 @@ protected override bool IsValidOpeningBraceToken(SyntaxToken token) protected override bool IsValidClosingBraceToken(SyntaxToken token) => token.IsKind(SyntaxKind.GreaterThanToken); - protected override ValueTask IsValidOpenBraceTokenAtPositionAsync(Document document, SyntaxToken token, int position, CancellationToken cancellationToken) + protected override async ValueTask IsValidOpenBraceTokenAtPositionAsync(Document document, SyntaxToken token, int position, CancellationToken cancellationToken) { + cancellationToken.ThrowIfCancellationRequested(); + // check what parser thinks about the newly typed "<" and only proceed if parser thinks it is "<" of // type argument or parameter list if (token.CheckParent(n => n.LessThanToken == token) || token.CheckParent(n => n.LessThanToken == token) || token.CheckParent(n => n.LessThanToken == token)) { - return ValueTaskFactory.FromResult(true); + return true; } // type argument can be easily ambiguous with normal < operations if (token.Parent is not BinaryExpressionSyntax(SyntaxKind.LessThanExpression) node || node.OperatorToken != token) - return ValueTaskFactory.FromResult(false); + return false; // type_argument_list only shows up in the following grammar construct: // @@ -58,15 +60,10 @@ protected override ValueTask IsValidOpenBraceTokenAtPositionAsync(Document // So if the prior token is not an identifier, this could not be a type-argument-list. var previousToken = token.GetPreviousToken(); if (previousToken.Parent is not IdentifierNameSyntax identifier) - return ValueTaskFactory.FromResult(false); - - return IsSemanticTypeArgumentAsync(document, node.SpanStart, identifier, cancellationToken); + return false; - static async ValueTask IsSemanticTypeArgumentAsync(Document document, int position, IdentifierNameSyntax identifier, CancellationToken cancellationToken) - { - var semanticModel = await document.ReuseExistingSpeculativeModelAsync(position, cancellationToken).ConfigureAwait(false); - var info = semanticModel.GetSymbolInfo(identifier, cancellationToken); - return info.CandidateSymbols.Any(static s => s.GetArity() > 0); - } + var semanticModel = await document.ReuseExistingSpeculativeModelAsync(position, cancellationToken).ConfigureAwait(false); + var info = semanticModel.GetSymbolInfo(identifier, cancellationToken); + return info.CandidateSymbols.Any(static s => s.GetArity() > 0); } } diff --git a/src/Features/Core/Portable/BraceCompletion/AbstractBraceCompletionService.cs b/src/Features/Core/Portable/BraceCompletion/AbstractBraceCompletionService.cs index bd58c7d7b3f0a..52f3c943fefd4 100644 --- a/src/Features/Core/Portable/BraceCompletion/AbstractBraceCompletionService.cs +++ b/src/Features/Core/Portable/BraceCompletion/AbstractBraceCompletionService.cs @@ -40,6 +40,7 @@ internal abstract class AbstractBraceCompletionService : IBraceCompletionService public ValueTask HasBraceCompletionAsync(BraceCompletionContext context, Document document, CancellationToken cancellationToken) { + cancellationToken.ThrowIfCancellationRequested(); if (!context.HasCompletionForOpeningBrace(OpeningBrace)) return ValueTaskFactory.FromResult(false); From 21ea7f704f91841cb3c52eaa6f52fbca99997091 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 14 Jan 2025 13:02:11 -0800 Subject: [PATCH 2/3] Simplify --- .../BraceCompletionSessionProvider.BraceCompletionSession.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/EditorFeatures/Core/AutomaticCompletion/BraceCompletionSessionProvider.BraceCompletionSession.cs b/src/EditorFeatures/Core/AutomaticCompletion/BraceCompletionSessionProvider.BraceCompletionSession.cs index 2bf95ff15ea25..161f684e774cd 100644 --- a/src/EditorFeatures/Core/AutomaticCompletion/BraceCompletionSessionProvider.BraceCompletionSession.cs +++ b/src/EditorFeatures/Core/AutomaticCompletion/BraceCompletionSessionProvider.BraceCompletionSession.cs @@ -68,15 +68,14 @@ public void Start() { ThreadingContext.ThrowIfNotOnUIThread(); + // Brace completion is cancellable if the user has the 'responsive completion' option enabled. var cancellationTokenSource = new CancellationTokenSource(); - var cancellationToken = cancellationTokenSource.Token; - if (_timeoutThreshold > 0) cancellationTokenSource.CancelAfter(_timeoutThreshold); try { - var success = ThreadingContext.JoinableTaskFactory.Run(() => TryStartAsync(cancellationToken)); + var success = ThreadingContext.JoinableTaskFactory.Run(() => TryStartAsync(cancellationTokenSource.Token)); if (!success) EndSession(); } From a3e40fe40beef0bb8bc54cc0c92995330a192823 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 14 Jan 2025 13:09:52 -0800 Subject: [PATCH 3/3] Simplify --- ...mpletionSessionProvider.BraceCompletionSession.cs | 12 +++++++----- .../BraceCompletionSessionProvider.cs | 5 ++--- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/EditorFeatures/Core/AutomaticCompletion/BraceCompletionSessionProvider.BraceCompletionSession.cs b/src/EditorFeatures/Core/AutomaticCompletion/BraceCompletionSessionProvider.BraceCompletionSession.cs index 161f684e774cd..dfde00cdddd5e 100644 --- a/src/EditorFeatures/Core/AutomaticCompletion/BraceCompletionSessionProvider.BraceCompletionSession.cs +++ b/src/EditorFeatures/Core/AutomaticCompletion/BraceCompletionSessionProvider.BraceCompletionSession.cs @@ -41,14 +41,14 @@ private class BraceCompletionSession( char closingBrace, ITextUndoHistory undoHistory, IBraceCompletionService service, - int timeoutThreshold) : IBraceCompletionSession + bool responsiveCompletion) : IBraceCompletionSession { private readonly BraceCompletionSessionProvider _provider = provider; private readonly IEditorOperations _editorOperations = provider._editorOperationsFactoryService.GetEditorOperations(textView); private readonly IBraceCompletionService _service = service; private readonly ITextUndoHistory _undoHistory = undoHistory; - private readonly int _timeoutThreshold = timeoutThreshold; + private readonly bool _responsiveCompletion = responsiveCompletion; private IThreadingContext ThreadingContext => _provider._threadingContext; private EditorOptionsService EditorOptionsService => _provider._editorOptionsService; @@ -68,10 +68,12 @@ public void Start() { ThreadingContext.ThrowIfNotOnUIThread(); - // Brace completion is cancellable if the user has the 'responsive completion' option enabled. + // Brace completion is cancellable if the user has the 'responsive completion' option enabled. 200 ms was + // chosen as the default timeout with the editor as a good balance of having enough time for computation, + // while canceling early enough to not be too disruptive. var cancellationTokenSource = new CancellationTokenSource(); - if (_timeoutThreshold > 0) - cancellationTokenSource.CancelAfter(_timeoutThreshold); + if (_responsiveCompletion) + cancellationTokenSource.CancelAfter(200); try { diff --git a/src/EditorFeatures/Core/AutomaticCompletion/BraceCompletionSessionProvider.cs b/src/EditorFeatures/Core/AutomaticCompletion/BraceCompletionSessionProvider.cs index 9dccc2c4981c0..9335c59b9ac59 100644 --- a/src/EditorFeatures/Core/AutomaticCompletion/BraceCompletionSessionProvider.cs +++ b/src/EditorFeatures/Core/AutomaticCompletion/BraceCompletionSessionProvider.cs @@ -48,8 +48,7 @@ public bool TryCreateSession(ITextView textView, SnapshotPoint openingPoint, cha { _threadingContext.ThrowIfNotOnUIThread(); - var isResponsive = textView.Options.GetOptionValue(DefaultOptions.ResponsiveCompletionOptionId); - var responsiveThreshold = isResponsive ? textView.Options.GetOptionValue(DefaultOptions.ResponsiveCompletionThresholdOptionId) : -1; + var responsiveCompletion = textView.Options.GetOptionValue(DefaultOptions.ResponsiveCompletionOptionId); var textSnapshot = openingPoint.Snapshot; var document = textSnapshot.GetOpenDocumentInCurrentContextWithChanges(); @@ -69,7 +68,7 @@ public bool TryCreateSession(ITextView textView, SnapshotPoint openingPoint, cha session = new BraceCompletionSession( this, textView, openingPoint.Snapshot.TextBuffer, openingPoint, openingBrace, closingBrace, - undoHistory, editorSession, responsiveThreshold); + undoHistory, editorSession, responsiveCompletion); return true; } }