Update the Copilot powered Inline Rename UX #76001
Conversation
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
dotnet-policy-service
bot
added
the
Community
| await Task.Delay(_smartRenameSession.AutomaticFetchDelay, cancellationToken) | ||
| .ConfigureAwait(false); | ||
| } | ||
| this.IsInPreparation = true; |
There was a problem hiding this comment.
i would like some sort of threading asserts here. can talk mroe on what we often do in roslyn.
There was a problem hiding this comment.
we should also assert that we're not already 'in preparation'.
There was a problem hiding this comment.
we should also assert that we're not already 'in preparation'.
This is already indirectly asserted above, through
if (_getSuggestionsTask.Status is TaskStatus.RanToCompletion or TaskStatus.Faulted or TaskStatus.Canceled)
{
...
_getSuggestionsTask = GetSuggestionsTaskAsync(isAutomaticOnInitialization, _cancellationTokenSource.Token).CompletesAsyncOperation(listenerToken);
}
Can you elaborate on threading asserts? This method should be free threaded; I believe it starts on UI thread (asserted in FetchSuggestions, because FetchSuggestions needs to avoid races in setting _getSuggestionsTask)
There was a problem hiding this comment.
This is already indirectly asserted above, through
I mean a literal retail assert that IsInPreparation must be false before setting it to true. Alternative, this can be in the setter, which retail asserts that the value is always flipping.
There was a problem hiding this comment.
Can you elaborate on threading asserts? This method should be free threaded;
It is not ok to be free threaded. two threads might call in here, and end up with one setting the boolean back to false, while the other is still running.
src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs
Outdated
Show resolved
Hide resolved
| else | ||
| { | ||
| _cancellationTokenSource?.Cancel(); | ||
| } |
There was a problem hiding this comment.
how can the cts ever be null?
There was a problem hiding this comment.
Before FetchSuggestions is ever called. It's defined as a nullable field.
There was a problem hiding this comment.
Very odd. I would expect it to just be 'new'ed up when we instantiate this type. :)
There is a slight delay prior to making Copilot request. Additionally, Roslyn might be gathering references and definition to add semantic context to the prompt.
Our UX designer recommended that we should be showing the progress bar while this is happening.
Before:
After:
Additionally, I saw that pressing Ctrl+Space to cancel ongoing request did not work, so I fixed it. In the "before" screenshot, I hit Ctrl+Space to cancel the request, then arrow down to scroll through suggestions, which were not visible
Before:
After:
