Skip to content

Commit 0b51e07

Browse files
rkeithhillTylerLeonhardt
authored andcommitted
Fix unable to open files in problems/peek windows issue (#872)
* Fix unable to open files in problems/peek windows issue This is due to PSES not properly storing a language client path in the ClientFilePath property of ScriptFile. This change ensures that a ClientFilePath is always stored in the text document Uri that a LSP client expects. This code has mostly been provided by @SeeminglyScience. Thanks for the reference to your code. This fixes issue 1732 in the vscode-powershell repo. * Play it safe by adding a new property instead of changing ClientPath * Address Codacy issues * Add tests for DocumentUri and fix colon issue on Linux/macOS * Attempt to fix where we replace colons on Linux * Try to fix tests for Linux * Address PR feedback * Kick the build * Address PR feedback, use StringBuilder.Replace() * Add test for backslash char on Linux/macOS. * Update src/PowerShellEditorServices/Workspace/Workspace.cs Co-Authored-By: rkeithhill <[email protected]>
1 parent c9d4987 commit 0b51e07

File tree

8 files changed

+120
-14
lines changed

8 files changed

+120
-14
lines changed

src/PowerShellEditorServices.Host/CodeLens/CodeLensFeature.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ private async Task HandleCodeLensRequestAsync(
126126
codeLensResponse[i] = codeLensResults[i].ToProtocolCodeLens(
127127
new CodeLensData
128128
{
129-
Uri = codeLensResults[i].File.ClientFilePath,
129+
Uri = codeLensResults[i].File.DocumentUri,
130130
ProviderId = codeLensResults[i].Provider.ProviderId
131131
},
132132
_jsonSerializer);

src/PowerShellEditorServices.Host/CodeLens/PesterCodeLensProvider.cs

+4-6
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,11 @@
33
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
44
//
55

6-
using Microsoft.PowerShell.EditorServices.Commands;
7-
using Microsoft.PowerShell.EditorServices.Symbols;
8-
using System;
96
using System.Collections.Generic;
10-
using System.Linq;
117
using System.Threading;
128
using System.Threading.Tasks;
9+
using Microsoft.PowerShell.EditorServices.Commands;
10+
using Microsoft.PowerShell.EditorServices.Symbols;
1311

1412
namespace Microsoft.PowerShell.EditorServices.CodeLenses
1513
{
@@ -53,7 +51,7 @@ private CodeLens[] GetPesterLens(PesterSymbolReference pesterSymbol, ScriptFile
5351
"PowerShell.RunPesterTests",
5452
"Run tests",
5553
new object[] {
56-
scriptFile.ClientFilePath,
54+
scriptFile.DocumentUri,
5755
false /* No debug */,
5856
pesterSymbol.TestName,
5957
pesterSymbol.ScriptRegion?.StartLineNumber })),
@@ -66,7 +64,7 @@ private CodeLens[] GetPesterLens(PesterSymbolReference pesterSymbol, ScriptFile
6664
"PowerShell.RunPesterTests",
6765
"Debug tests",
6866
new object[] {
69-
scriptFile.ClientFilePath,
67+
scriptFile.DocumentUri,
7068
true /* Run in the debugger */,
7169
pesterSymbol.TestName,
7270
pesterSymbol.ScriptRegion?.StartLineNumber })),

src/PowerShellEditorServices.Host/CodeLens/ReferencesCodeLensProvider.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ public async Task<CodeLens> ResolveCodeLensAsync(
118118
GetReferenceCountHeader(referenceLocations.Length),
119119
new object[]
120120
{
121-
codeLens.File.ClientFilePath,
121+
codeLens.File.DocumentUri,
122122
codeLens.ScriptExtent.ToRange().Start,
123123
referenceLocations,
124124
}

src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1757,15 +1757,15 @@ private static async Task PublishScriptDiagnosticsAsync(
17571757
diagnostics.Add(markerDiagnostic);
17581758
}
17591759

1760-
correctionIndex[scriptFile.ClientFilePath] = fileCorrections;
1760+
correctionIndex[scriptFile.DocumentUri] = fileCorrections;
17611761

17621762
// Always send syntax and semantic errors. We want to
17631763
// make sure no out-of-date markers are being displayed.
17641764
await eventSender(
17651765
PublishDiagnosticsNotification.Type,
17661766
new PublishDiagnosticsNotification
17671767
{
1768-
Uri = scriptFile.ClientFilePath,
1768+
Uri = scriptFile.DocumentUri,
17691769
Diagnostics = diagnostics.ToArray()
17701770
});
17711771
}

src/PowerShellEditorServices/Workspace/ScriptFile.cs

+15-1
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,14 @@
33
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
44
//
55

6-
using Microsoft.PowerShell.EditorServices.Utility;
76
using System;
87
using System.Collections.Generic;
98
using System.IO;
109
using System.Linq;
1110
using System.Management.Automation;
1211
using System.Management.Automation.Language;
12+
using System.Runtime.InteropServices;
13+
using Microsoft.PowerShell.EditorServices.Utility;
1314

1415
namespace Microsoft.PowerShell.EditorServices
1516
{
@@ -52,6 +53,19 @@ public string Id
5253
/// </summary>
5354
public string ClientFilePath { get; private set; }
5455

56+
/// <summary>
57+
/// Gets the file path in LSP DocumentUri form. The ClientPath property must not be null.
58+
/// </summary>
59+
public string DocumentUri
60+
{
61+
get
62+
{
63+
return this.ClientFilePath == null
64+
? string.Empty
65+
: Workspace.ConvertPathToDocumentUri(this.ClientFilePath);
66+
}
67+
}
68+
5569
/// <summary>
5670
/// Gets or sets a boolean that determines whether
5771
/// semantic analysis should be enabled for this file.

src/PowerShellEditorServices/Workspace/Workspace.cs

+65-2
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,7 @@ private void RecursivelyEnumerateFiles(string folderPath, ref List<string> found
351351
this.logger.WriteHandledException(
352352
$"Could not enumerate files in the path '{folderPath}' due to an exception",
353353
e);
354-
354+
355355
continue;
356356
}
357357

@@ -400,7 +400,7 @@ private void RecursivelyEnumerateFiles(string folderPath, ref List<string> found
400400
this.logger.WriteHandledException(
401401
$"Could not enumerate directories in the path '{folderPath}' due to an exception",
402402
e);
403-
403+
404404
return;
405405
}
406406

@@ -625,6 +625,69 @@ private static string UnescapeDriveColon(string fileUri)
625625
return sb.ToString();
626626
}
627627

628+
/// <summary>
629+
/// Converts a file system path into a DocumentUri required by Language Server Protocol.
630+
/// </summary>
631+
/// <remarks>
632+
/// When sending a document path to a LSP client, the path must be provided as a
633+
/// DocumentUri in order to features like the Problems window or peek definition
634+
/// to be able to open the specified file.
635+
/// </remarks>
636+
/// <param name="path">
637+
/// A file system path. Note: if the path is already a DocumentUri, it will be returned unmodified.
638+
/// </param>
639+
/// <returns>The file system path encoded as a DocumentUri.</returns>
640+
internal static string ConvertPathToDocumentUri(string path)
641+
{
642+
const string fileUriPrefix = "file:///";
643+
644+
if (path.StartsWith("untitled:", StringComparison.Ordinal))
645+
{
646+
return path;
647+
}
648+
649+
if (path.StartsWith(fileUriPrefix, StringComparison.Ordinal))
650+
{
651+
return path;
652+
}
653+
654+
if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
655+
{
656+
// On a Linux filesystem, you can have multiple colons in a filename e.g. foo:bar:baz.txt
657+
string absoluteUri = new Uri(path).AbsoluteUri;
658+
659+
// First colon is part of the protocol scheme, see if there are other colons in the path
660+
int firstColonIndex = absoluteUri.IndexOf(':');
661+
if (absoluteUri.IndexOf(':', firstColonIndex + 1) > firstColonIndex)
662+
{
663+
absoluteUri = new StringBuilder(absoluteUri)
664+
.Replace(
665+
oldValue: ":",
666+
newValue: "%3A",
667+
startIndex: firstColonIndex + 1,
668+
count: absoluteUri.Length - firstColonIndex - 1)
669+
.ToString();
670+
}
671+
672+
return absoluteUri;
673+
}
674+
675+
// VSCode file URIs on Windows need the drive letter lowercase, and the colon
676+
// URI encoded. System.Uri won't do that, so we manually create the URI.
677+
var newUri = new StringBuilder(System.Web.HttpUtility.UrlPathEncode(path));
678+
int colonIndex = path.IndexOf(':');
679+
if (colonIndex > 0)
680+
{
681+
int driveLetterIndex = colonIndex - 1;
682+
char driveLetter = char.ToLowerInvariant(path[driveLetterIndex]);
683+
newUri
684+
.Replace(path[driveLetterIndex], driveLetter, driveLetterIndex, 1)
685+
.Replace(":", "%3A", colonIndex, 1);
686+
}
687+
688+
return newUri.Replace('\\', '/').Insert(0, fileUriPrefix).ToString();
689+
}
690+
628691
#endregion
629692
}
630693
}

test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ public static IEnumerable<object[]> DebuggerAcceptsScriptArgsTestData
101101
}
102102

103103
[Theory]
104-
[MemberData("DebuggerAcceptsScriptArgsTestData")]
104+
[MemberData(nameof(DebuggerAcceptsScriptArgsTestData))]
105105
public async Task DebuggerAcceptsScriptArgs(string[] args)
106106
{
107107
// The path is intentionally odd (some escaped chars but not all) because we are testing

test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs

+31
Original file line numberDiff line numberDiff line change
@@ -572,6 +572,7 @@ public void PropertiesInitializedCorrectlyForUntitled()
572572

573573
Assert.Equal(path, scriptFile.FilePath);
574574
Assert.Equal(path, scriptFile.ClientFilePath);
575+
Assert.Equal(path, scriptFile.DocumentUri);
575576
Assert.True(scriptFile.IsAnalysisEnabled);
576577
Assert.True(scriptFile.IsInMemory);
577578
Assert.Empty(scriptFile.ReferencedFiles);
@@ -580,5 +581,35 @@ public void PropertiesInitializedCorrectlyForUntitled()
580581
Assert.Equal(3, scriptFile.FileLines.Count);
581582
}
582583
}
584+
585+
[Fact]
586+
public void DocumentUriRetunsCorrectStringForAbsolutePath()
587+
{
588+
string path;
589+
ScriptFile scriptFile;
590+
var emptyStringReader = new StringReader("");
591+
592+
if (Environment.OSVersion.Platform == PlatformID.Win32NT)
593+
{
594+
path = @"C:\Users\AmosBurton\projects\Rocinate\ProtoMolecule.ps1";
595+
scriptFile = new ScriptFile(path, path, emptyStringReader, PowerShellVersion);
596+
Assert.Equal("file:///c%3A/Users/AmosBurton/projects/Rocinate/ProtoMolecule.ps1", scriptFile.DocumentUri);
597+
}
598+
else
599+
{
600+
// Test the following only on Linux and macOS.
601+
path = "/home/AlexKamal/projects/Rocinate/ProtoMolecule.ps1";
602+
scriptFile = new ScriptFile(path, path, emptyStringReader, PowerShellVersion);
603+
Assert.Equal("file:///home/AlexKamal/projects/Rocinate/ProtoMolecule.ps1", scriptFile.DocumentUri);
604+
605+
path = "/home/NaomiNagata/projects/Rocinate/Proto:Mole:cule.ps1";
606+
scriptFile = new ScriptFile(path, path, emptyStringReader, PowerShellVersion);
607+
Assert.Equal("file:///home/NaomiNagata/projects/Rocinate/Proto%3AMole%3Acule.ps1", scriptFile.DocumentUri);
608+
609+
path = "/home/JamesHolden/projects/Rocinate/Proto:Mole\\cule.ps1";
610+
scriptFile = new ScriptFile(path, path, emptyStringReader, PowerShellVersion);
611+
Assert.Equal("file:///home/JamesHolden/projects/Rocinate/Proto%3AMole%5Ccule.ps1", scriptFile.DocumentUri);
612+
}
613+
}
583614
}
584615
}

0 commit comments

Comments
 (0)