Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -391,10 +391,15 @@ private void UpdateProjectDocuments(

_logger.LogTrace($"Updating document '{newHostDocument.FilePath}''s file kind to '{newHostDocument.FileKind}' and target path to '{newHostDocument.TargetPath}'.");

var remoteTextLoader = _remoteTextLoaderFactory.Create(newFilePath);
// If the physical file name hasn't changed, we use the current document snapshot as the source of truth for text, in case
// it has received text change info from LSP. eg, if someone changes the TargetPath of the file while its open in the editor
// with unsaved changes, we don't want to reload it from disk.
var textLoader = FilePathComparer.Instance.Equals(currentHostDocument.FilePath, newHostDocument.FilePath)
? new DocumentSnapshotTextLoader(documentSnapshot)
: _remoteTextLoaderFactory.Create(newFilePath);

updater.DocumentRemoved(currentProjectKey, currentHostDocument);
updater.DocumentAdded(currentProjectKey, newHostDocument, remoteTextLoader);
updater.DocumentAdded(currentProjectKey, newHostDocument, textLoader);
}

project = _projectManager.GetLoadedProject(project.Key);
Expand Down Expand Up @@ -493,18 +498,27 @@ private void TryMigrateMiscellaneousDocumentsToProject(ProjectSnapshotManager.Up
}

// Remove from miscellaneous project
var defaultMiscProject = miscellaneousProject;

updater.DocumentRemoved(defaultMiscProject.Key, documentSnapshot.State.HostDocument);
updater.DocumentRemoved(miscellaneousProject.Key, documentSnapshot.State.HostDocument);

// Add to new project

var textLoader = new DocumentSnapshotTextLoader(documentSnapshot);
var defaultProject = projectSnapshot;
var newHostDocument = new HostDocument(documentSnapshot.FilePath, documentSnapshot.TargetPath);

// If we're moving from the misc files project to a real project, then target path will be the full path to the file
// and the next update to the project will update it to be a relative path. To save a bunch of busy work if that is
// the only change necessary, we can proactively do that work here. This also means that when we later find out about
// this document the "real" way, it will be equal to the one we already know about, and we won't lose content
var projectDirectory = FilePathNormalizer.GetNormalizedDirectoryName(projectSnapshot.FilePath);
var newTargetPath = documentSnapshot.TargetPath;
if (FilePathNormalizer.Normalize(newTargetPath).StartsWith(projectDirectory))
{
newTargetPath = newTargetPath[projectDirectory.Length..];
}

var newHostDocument = new HostDocument(documentSnapshot.FilePath, newTargetPath, documentSnapshot.FileKind);
Copy link
Contributor

@maryamariyan maryamariyan May 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that using the other HostDocument ctor may cause the logic to set FileKind to change, hopefully that's okay. (will no longer be null)

public HostDocument(string filePath, string targetPath)
: this(filePath, targetPath, fileKind: null)
{
}
public HostDocument(string filePath, string targetPath, string? fileKind)
{
FilePath = filePath ?? throw new ArgumentNullException(nameof(filePath));
TargetPath = targetPath ?? throw new ArgumentNullException(nameof(targetPath));
FileKind = fileKind ?? FileKinds.GetFileKindFromFilePath(filePath);
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other constructor determines the file kind from the file path, and it seemed unnecessary to do that if we already know the file kind. If the file kind didn't match the file path (ie, somehow we had a .razor file with a kind of "mvc") then the previous code would have "fixed" it, whereas the current code will keep it, but I honestly don't know which of those situations is right or wrong.

_logger.LogInformation($"Migrating '{documentFilePath}' from the '{miscellaneousProject.Key}' project to '{projectSnapshot.Key}' project.");

updater.DocumentAdded(defaultProject.Key, newHostDocument, textLoader);
updater.DocumentAdded(projectSnapshot.Key, newHostDocument, textLoader);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT license. See License.txt in the project root for license information.

using System;
using System.Collections.Immutable;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
Expand Down Expand Up @@ -1137,13 +1138,77 @@ await _projectManager.UpdateAsync(updater =>
x => x.DocumentRemoved(DocumentFilePath2, miscProject.Key));
}

private static TextLoader CreateEmptyTextLoader()
[Fact]
public async Task AddProject_MigratesMiscellaneousDocumentsToNewOwnerProject_FixesTargetPath()
{
// Arrange
const string ProjectFilePath = "C:/path/to/project.csproj";
const string IntermediateOutputPath = "C:/path/to/obj";
const string DocumentFilePath1 = "C:/path/to/document1.cshtml";
const string DocumentFilePath2 = "C:/path/to/document2.cshtml";

var miscProject = _snapshotResolver.GetMiscellaneousProject();

await _projectManager.UpdateAsync(updater =>
{
updater.DocumentAdded(miscProject.Key,
new HostDocument(DocumentFilePath1, "C:/path/to/document1.cshtml"), CreateEmptyTextLoader());
updater.DocumentAdded(miscProject.Key,
new HostDocument(DocumentFilePath2, "C:/path/to/document2.cshtml"), CreateEmptyTextLoader());
});

// Act
var newProjectKey = await _projectService.AddProjectAsync(
ProjectFilePath, IntermediateOutputPath, RazorConfiguration.Default, rootNamespace: null, displayName: null, DisposalToken);

// Assert
var newProject = _projectManager.GetLoadedProject(newProjectKey);
Assert.Equal("document1.cshtml", newProject.GetDocument(DocumentFilePath1)!.TargetPath);
Assert.Equal("document2.cshtml", newProject.GetDocument(DocumentFilePath2)!.TargetPath);
}

[Fact]
public async Task AddOrUpdateProjectAsync_MigratesMiscellaneousDocumentsToNewOwnerProject_MaintainsTextState()
{
// Arrange
const string ProjectFilePath = "C:/path/to/project.csproj";
const string IntermediateOutputPath = "C:/path/to/obj";
const string DocumentFilePath1 = "C:/path/to/document1.cshtml";

var miscProject = _snapshotResolver.GetMiscellaneousProject();

await _projectManager.UpdateAsync(updater =>
{
updater.DocumentAdded(miscProject.Key,
new HostDocument(DocumentFilePath1, "other/document1.cshtml"), CreateTextLoader(SourceText.From("Hello")));
});

var projectKey = new ProjectKey(IntermediateOutputPath);

var documentHandles = ImmutableArray.Create(new DocumentSnapshotHandle(DocumentFilePath1, "document1.cshtml", "mvc"));

// Act
await _projectService.AddOrUpdateProjectAsync(
projectKey, ProjectFilePath, RazorConfiguration.Default, rootNamespace: null, displayName: null, ProjectWorkspaceState.Default, documentHandles, DisposalToken);

// Assert
var newProject = _projectManager.GetLoadedProject(projectKey);
var documentText = await newProject.GetDocument(DocumentFilePath1)!.GetTextAsync();
Assert.Equal("Hello", documentText.ToString());
}

private static TextLoader CreateTextLoader(SourceText text)
{
var textLoaderMock = new StrictMock<TextLoader>();
textLoaderMock
.Setup(x => x.LoadTextAndVersionAsync(It.IsAny<LoadTextOptions>(), It.IsAny<CancellationToken>()))
.ReturnsAsync(TextAndVersion.Create(s_emptyText, VersionStamp.Create()));
.ReturnsAsync(TextAndVersion.Create(text, VersionStamp.Create()));

return textLoaderMock.Object;
}

private static TextLoader CreateEmptyTextLoader()
{
return CreateTextLoader(s_emptyText);
}
}