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.

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

Add filePath when generates new Document in MoveTypeEditor #68900

Closed
wants to merge 5 commits into from

Conversation

Cosifne
Copy link
Member

@Cosifne Cosifne commented Jul 6, 2023

Fix #68550
MoveTypeEditor generate a new Document by providing

  1. The containing folders of the old document
  2. The new name of the document.
    This is not a problem in VS because we check if the folders exist https://sourceroslyn.io/#Microsoft.VisualStudio.LanguageServices/ProjectSystem/VisualStudioWorkspaceImpl.cs,793, then the document would be added to the folder.

However, in VS Code,

document => _projectSystemProject.AddSourceFile(document.FilePath),
when project system loads the document, it doesn't provide containing folder information. And we also don't check that when add a document to project. (We only check the file path, and if it doesn't exist, fall back to the project root)

So we can fix the problem in two ways.

  1. Parse the folders for all the documents in LSP, and also checking the containing folder when add documents in CodeActionHandler. And we don't need to change the MoveTypeEditor.
  2. Let the MoveTypeEditor gives more information about the file path.

This PR fix the problem in the second way.

@Cosifne Cosifne requested a review from a team as a code owner July 6, 2023 19:56
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Jul 6, 2023
@jasonmalinowski
Copy link
Member

@Cosifne: if we just implemented folder information in VS Code, would that make this problem go away?

@Cosifne
Copy link
Member Author

Cosifne commented Jul 7, 2023

@Cosifne: if we just implemented folder information in VS Code, would that make this problem go away?

@jasonmalinowski
If the folder information is provided in VSCode, we need to add one more extension method here


e.g.

  1. Check file path.
  2. If file path is null, check containing folder. (This one is missing now)
  3. Fall back to project root.

@jasonmalinowski
Copy link
Member

@Cosifne We can close this in favor of #68995, right? I'm liking that approach so we should go ahead with that I think.

@Cosifne
Copy link
Member Author

Cosifne commented Jul 25, 2023

@Cosifne We can close this in favor of #68995, right? I'm liking that approach so we should go ahead with that I think.

Sure, I keep forgetting this PR 😅

@Cosifne Cosifne closed this Jul 25, 2023
@Cosifne Cosifne deleted the dev/shech/FixMoveTypeInDirectory branch July 25, 2023 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Move type to" code action using C# Dev Kit incorrectly moves the type to the root folder.
3 participants