Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Inline rename couldn't rename a file #56467

Closed
davidwengier opened this issue Sep 16, 2021 · 6 comments · Fixed by #57725
Closed

Inline rename couldn't rename a file #56467

davidwengier opened this issue Sep 16, 2021 · 6 comments · Fixed by #57725
Assignees
@ryzngard
Labels
Area-IDE Bug
Milestone
17.1

Comments

@davidwengier
Copy link
Member

davidwengier commented Sep 16, 2021 •
edited
Loading

Version Used: Version 17.0.0 Preview 5.0 [31715.371.main]

image

System.Exception : Unable to check out the files from source control.
   at Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem.VisualStudioWorkspaceImpl.EnsureEditableDocuments(IEnumerable`1 documents)
   at Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem.VisualStudioWorkspaceImpl.ApplyTextDocumentChange(DocumentId documentId,SourceText newText)
   at Microsoft.CodeAnalysis.Workspace.ApplyChangedDocument(ProjectChanges projectChanges,DocumentId documentId)
   at Microsoft.CodeAnalysis.Workspace.ApplyProjectChanges(ProjectChanges projectChanges)
   at Microsoft.CodeAnalysis.Workspace.TryApplyChanges(Solution newSolution,IProgressTracker progressTracker)
   at Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem.VisualStudioWorkspaceImpl.TryApplyChanges(Solution newSolution,IProgressTracker progressTracker)
   at Microsoft.CodeAnalysis.Editor.Implementation.InlineRename.InlineRenameSession.ApplyRename(Solution newSolution,IUIThreadOperationContext operationContext)
   at Microsoft.CodeAnalysis.Editor.Implementation.InlineRename.InlineRenameSession.CommitCore(IUIThreadOperationContext operationContext,Boolean previewChanges)
   at Microsoft.VisualStudio.Editor.Implementation.VSUIThreadOperationExecutor.Execute(UIThreadOperationExecutionOptions executionOptions,Action`1 action)
   at Microsoft.CodeAnalysis.Editor.Implementation.InlineRename.InlineRenameSession.CommitWorker(Boolean previewChanges)
   at Microsoft.CodeAnalysis.Editor.Implementation.InlineRename.RenameCommandHandler.Commit(InlineRenameSession activeSession,ITextView textView)
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at Microsoft.VisualStudio.Telemetry.WindowsErrorReporting.WatsonReport.GetClrWatsonExceptionInfo(Exception exceptionObject)
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Sep 16, 2021
@jinujoseph jinujoseph added Bug and removed untriaged Issues and PRs which have not yet been triaged by a lead labels Sep 22, 2021
@jinujoseph jinujoseph added this to the 17.1 milestone Sep 22, 2021
@ryzngard
Copy link
Contributor

@genlu we're hitting this multiple times. Can we revisit this design? What was the context we originally needed to have Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem.VisualStudioWorkspaceImpl.EnsureEditableDocuments ?

@genlu
Copy link
Member

genlu commented Sep 22, 2021 •
edited
Loading

I remember making changes to exclude editable document by querying IDocumentService, but it happens before passing documents for actual edits. The error here is caused by querying a VS service, which is separate from IDocumentService and I don't know anything about. Maybe we should sync with the git provider team to discuss a potential mitigation for this.

@ryzngard
Copy link
Contributor

@genlu I see... IMO it'd be nice to just remove this check. The only thing that will happen without it is that the change will fail if it actually can't be modified. Having this check only adds another point of failure for us.

@genlu
Copy link
Member

genlu commented Sep 28, 2021

Maybe showing a warning in goldbar and skip the problematic file, instead of removing the check completely?

@DavidKarlas
Copy link
Contributor

DavidKarlas commented Nov 2, 2021 •
edited
Loading

@davidwengier let me guess, that file is referenced in multiple projects/multitargeting?
This is duplicate of #45031 that I listed nice repro steps and explanation of bug...
#45031 (comment)

@davidwengier
Copy link
Member Author

I can't remember exactly, but looking at the nav bar, yes it seems like it would have been a multi-targeted project.

This was referenced Nov 12, 2021
Merged
Closed
ryzngard added a commit that referenced this issue Nov 12, 2021
@ryzngard
Remove extra EnsureEditableDocuments calls (#57725)
a4d1d8f 
Remove single check for EnsureEditableDocuments on each document. This was duplicating the work that the call in TryApplyChanges does, and can have conflicts with linked files.

For example:

If file A.cs is renamed to B.cs and is in 2 projects
Each project will go through and apply changes (lets call P1 and P2)
P1 goes through and calls: ApplyTextChange > EnsureEditable > Rename File (File is now B.cs on disk)
P2 goes through to do the same as P1, but A.cs no longer exists so EnsureEditable will fail (can't check status of a file that doesn't exist)
We should be doing all the work to check edit status upfront so it doesn't collide with the actual edits we need to do. TryApplyChanges is already overridden and does that. This change just removes the extra work we were doing that also created problems for renaming files.

Fixes #56467
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ryzngard ryzngard

Labels
Area-IDE Bug
Projects
None yet
Milestone
17.1
Development

Successfully merging a pull request may close this issue.

Remove extra EnsureEditableDocuments calls ryzngard/roslyn
5 participants
@ryzngard @davidwengier @DavidKarlas @genlu @jinujoseph