Skip to content

Commit

Permalink
Allow WorkspacePaths to be empty if we're not in a workspace
Browse files Browse the repository at this point in the history
We were previously adding the initial working directory as a fallback if
there was no workspace. However, this led to new VS Code windows (not in
a workspace) enumerating all files in the home directory (if that's what
the client had to default to, without a workspace folder). And this
could even spam permission requests on more secure systems such as
macOS. So we just need to let be an empty enumerable.
  • Loading branch information
andyleejordan committed Nov 10, 2023
1 parent a691981 commit 0fa5751
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,8 @@ public async Task SaveFileAsync(string currentPath, string newSavePath)
}

// NOTE: This name is now outdated since we don't have a way to distinguish one workspace
// from another for the extension API.
// from another for the extension API. TODO: Should this be an empty string if we have no
// workspaces?
public string GetWorkspacePath() => _workspaceService.InitialWorkingDirectory;

public string[] GetWorkspacePaths() => _workspaceService.WorkspacePaths.ToArray();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,7 @@ public WorkspaceService(ILoggerFactory factory)

#region Public Methods

public IEnumerable<string> WorkspacePaths => WorkspaceFolders.Count == 0
? new List<string> { InitialWorkingDirectory }
: WorkspaceFolders.Select(i => i.Uri.GetFileSystemPath());
public IEnumerable<string> WorkspacePaths => WorkspaceFolders.Select(i => i.Uri.GetFileSystemPath());

/// <summary>
/// Gets an open file in the workspace. If the file isn't open but exists on the filesystem, load and return it.
Expand Down
34 changes: 21 additions & 13 deletions test/PowerShellEditorServices.Test/Session/WorkspaceTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
using System.Runtime.InteropServices;
using Microsoft.Extensions.Logging.Abstractions;
using Microsoft.PowerShell.EditorServices.Services;
using Microsoft.PowerShell.EditorServices.Test.Shared;
using Microsoft.PowerShell.EditorServices.Services.TextDocument;
using Xunit;
using Microsoft.PowerShell.EditorServices.Utility;
Expand All @@ -27,6 +26,10 @@ public class WorkspaceTests

internal static ScriptFile CreateScriptFile(string path) => new(path, "", VersionUtils.PSVersion);

// Remember that LSP does weird stuff to the drive letter, so we have to convert it to a URI
// and back to ensure that drive letter gets lower cased and everything matches up.
private static string s_workspacePath =>
DocumentUri.FromFileSystemPath(Path.GetFullPath("Fixtures/Workspace")).GetFileSystemPath();

[Fact]
public void CanResolveWorkspaceRelativePath()
Expand Down Expand Up @@ -76,16 +79,21 @@ internal static WorkspaceService FixturesWorkspace()
{
return new WorkspaceService(NullLoggerFactory.Instance)
{
InitialWorkingDirectory = TestUtilities.NormalizePath("Fixtures/Workspace")
WorkspaceFolders =
{
new WorkspaceFolder { Uri = DocumentUri.FromFileSystemPath(s_workspacePath) }
}
};
}

[Fact]
public void HasDefaultForWorkspacePaths()
{
WorkspaceService workspace = FixturesWorkspace();
string actual = Assert.Single(workspace.WorkspacePaths);
Assert.Equal(workspace.InitialWorkingDirectory, actual);
string workspacePath = Assert.Single(workspace.WorkspacePaths);
Assert.Equal(s_workspacePath, workspacePath);
// We shouldn't assume an initial working directory since none was given.
Assert.Null(workspace.InitialWorkingDirectory);
}

// These are the default values for the EnumeratePSFiles() method
Expand Down Expand Up @@ -129,18 +137,18 @@ public void CanRecurseDirectoryTree()

List<string> expected = new()
{
Path.Combine(workspace.InitialWorkingDirectory, "nested", "donotfind.ps1"),
Path.Combine(workspace.InitialWorkingDirectory, "nested", "nestedmodule.psd1"),
Path.Combine(workspace.InitialWorkingDirectory, "nested", "nestedmodule.psm1"),
Path.Combine(workspace.InitialWorkingDirectory, "rootfile.ps1")
Path.Combine(s_workspacePath, "nested", "donotfind.ps1"),
Path.Combine(s_workspacePath, "nested", "nestedmodule.psd1"),
Path.Combine(s_workspacePath, "nested", "nestedmodule.psm1"),
Path.Combine(s_workspacePath, "rootfile.ps1")
};

// .NET Core doesn't appear to use the same three letter pattern matching rule although the docs
// suggest it should be find the '.ps1xml' files because we search for the pattern '*.ps1'
// ref https://docs.microsoft.com/en-us/dotnet/api/system.io.directory.getfiles?view=netcore-2.1#System_IO_Directory_GetFiles_System_String_System_String_System_IO_EnumerationOptions_
if (RuntimeInformation.FrameworkDescription.StartsWith(".NET Framework"))
{
expected.Insert(3, Path.Combine(workspace.InitialWorkingDirectory, "other", "other.ps1xml"));
expected.Insert(3, Path.Combine(s_workspacePath, "other", "other.ps1xml"));
}

Assert.Equal(expected, actual);
Expand All @@ -157,7 +165,7 @@ public void CanRecurseDirectoryTreeWithLimit()
maxDepth: 1,
ignoreReparsePoints: s_defaultIgnoreReparsePoints
);
Assert.Equal(new[] { Path.Combine(workspace.InitialWorkingDirectory, "rootfile.ps1") }, actual);
Assert.Equal(new[] { Path.Combine(s_workspacePath, "rootfile.ps1") }, actual);
}

[Fact]
Expand All @@ -173,16 +181,16 @@ public void CanRecurseDirectoryTreeWithGlobs()
);

Assert.Equal(new[] {
Path.Combine(workspace.InitialWorkingDirectory, "nested", "nestedmodule.psd1"),
Path.Combine(workspace.InitialWorkingDirectory, "rootfile.ps1")
Path.Combine(s_workspacePath, "nested", "nestedmodule.psd1"),
Path.Combine(s_workspacePath, "rootfile.ps1")
}, actual);
}

[Fact]
public void CanOpenAndCloseFile()
{
WorkspaceService workspace = FixturesWorkspace();
string filePath = Path.GetFullPath(Path.Combine(workspace.InitialWorkingDirectory, "rootfile.ps1"));
string filePath = Path.GetFullPath(Path.Combine(s_workspacePath, "rootfile.ps1"));

ScriptFile file = workspace.GetFile(filePath);
Assert.Equal(workspace.GetOpenedFiles(), new[] { file });
Expand Down

0 comments on commit 0fa5751

Please sign in to comment.