From ae975a16e5f38e94cc63733b3363fda5523bb9a5 Mon Sep 17 00:00:00 2001 From: Keith Hill Date: Sat, 24 Mar 2018 23:19:54 -0600 Subject: [PATCH 1/4] Implement better way to test for in-memory file Fix #569 PSES would crash quite often when the user viewed a ps1 file in a diff window or an earlier version using GitLens. This approach using the System.Uri class that can detect file vs other schemes. URI chokes on relative file paths. In this case, we fall back to the previous approach of using Path.GetFullPath(). --- .../Workspace/Workspace.cs | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/src/PowerShellEditorServices/Workspace/Workspace.cs b/src/PowerShellEditorServices/Workspace/Workspace.cs index 3b5749f5e..018815eb6 100644 --- a/src/PowerShellEditorServices/Workspace/Workspace.cs +++ b/src/PowerShellEditorServices/Workspace/Workspace.cs @@ -390,22 +390,26 @@ internal static bool IsPathInMemory(string filePath) // view of the current file or an untitled file. try { - // NotSupportedException or ArgumentException gets thrown when - // given an invalid path. Since any non-standard path will - // trigger this, assume that it means it's an in-memory file - // unless the path starts with file: - Path.GetFullPath(filePath); + // File system absoulute paths will have a URI scheme of file:. + // Other schemes like "untitled:" and "gitlens-git:" will return false for IsFile. + var uri = new Uri(filePath); + isInMemory = !uri.IsFile; } - catch (ArgumentException) + catch (UriFormatException) { - isInMemory = true; - } - catch (NotSupportedException) - { - isInMemory = true; + // Relative file paths cause a UriFormatException. + // In this case, fallback to using Path.GetFullPath(). + try + { + Path.GetFullPath(filePath); + } + catch (Exception ex) when (ex is ArgumentException || ex is NotSupportedException) + { + isInMemory = true; + } } - return !filePath.ToLower().StartsWith("file:") && isInMemory; + return isInMemory; } private string GetBaseFilePath(string filePath) From 541822d5c38f2946eaa1b900732ffa8b6ed8dc06 Mon Sep 17 00:00:00 2001 From: Keith Hill Date: Sun, 25 Mar 2018 14:57:56 -0600 Subject: [PATCH 2/4] Add unit tests and PTLE catch - just in case. --- .../Workspace/Workspace.cs | 4 +++ .../Session/WorkspaceTests.cs | 30 +++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/src/PowerShellEditorServices/Workspace/Workspace.cs b/src/PowerShellEditorServices/Workspace/Workspace.cs index 018815eb6..7b192cfed 100644 --- a/src/PowerShellEditorServices/Workspace/Workspace.cs +++ b/src/PowerShellEditorServices/Workspace/Workspace.cs @@ -407,6 +407,10 @@ internal static bool IsPathInMemory(string filePath) { isInMemory = true; } + catch (PathTooLongException) + { + // If we ever get here, it should be an actual file so, not in memory + } } return isInMemory; diff --git a/test/PowerShellEditorServices.Test/Session/WorkspaceTests.cs b/test/PowerShellEditorServices.Test/Session/WorkspaceTests.cs index 4dfdabdc9..6e8025255 100644 --- a/test/PowerShellEditorServices.Test/Session/WorkspaceTests.cs +++ b/test/PowerShellEditorServices.Test/Session/WorkspaceTests.cs @@ -35,5 +35,35 @@ public void CanResolveWorkspaceRelativePath() Assert.Equal(@"..\PeerPath\FilePath.ps1", workspace.GetRelativePath(testPathOutside)); Assert.Equal(testPathAnotherDrive, workspace.GetRelativePath(testPathAnotherDrive)); } + + [Fact] + public void CanDetermineIsPathInMemory() + { + var tempDir = Environment.GetEnvironmentVariable("TEMP"); + + var shortDirPath = Path.Combine(tempDir, "GitHub", "PowerShellEditorServices"); + var shortFilePath = Path.Combine(shortDirPath, "foo.ps1"); + var shortDirUriForm = new Uri(shortDirPath).ToString(); + var shortFileUriForm = new Uri(shortFilePath).ToString(); + + // Test short file absolute paths + Assert.False(Workspace.IsPathInMemory(shortDirPath)); + Assert.False(Workspace.IsPathInMemory(shortFilePath)); + Assert.False(Workspace.IsPathInMemory(shortDirUriForm)); + Assert.False(Workspace.IsPathInMemory(shortFileUriForm)); + + // Test short file relative paths - not sure we'll ever get these but just in case + Assert.False(Workspace.IsPathInMemory("foo.ps1")); + Assert.False(Workspace.IsPathInMemory(".." + Path.DirectorySeparatorChar + "foo.ps1")); + + // Test short non-file paths + Assert.True(Workspace.IsPathInMemory("untitled:untitled-1")); + var shortUriForm = "git:/c%3A/Users/Keith/GitHub/dahlbyk/posh-git/src/PoshGitTypes.ps1?%7B%22path%22%3A%22c%3A%5C%5CUsers%5C%5CKeith%5C%5CGitHub%5C%5Cdahlbyk%5C%5Cposh-git%5C%5Csrc%5C%5CPoshGitTypes.ps1%22%2C%22ref%22%3A%22~%22%7D"; + Assert.True(Workspace.IsPathInMemory(shortUriForm)); + + // Test long non-file path - known to have crashed PSES + var longUriForm = "gitlens-git:c%3A%5CUsers%5CKeith%5CGitHub%5Cdahlbyk%5Cposh-git%5Csrc%5CPoshGitTypes%3Ae0022701.ps1?%7B%22fileName%22%3A%22src%2FPoshGitTypes.ps1%22%2C%22repoPath%22%3A%22c%3A%2FUsers%2FKeith%2FGitHub%2Fdahlbyk%2Fposh-git%22%2C%22sha%22%3A%22e0022701fa12e0bc22d0458673d6443c942b974a%22%7D"; + Assert.True(Workspace.IsPathInMemory(longUriForm)); + } } } From 6d3d665ac28a7fd9652a47ef57f18878fa4452e3 Mon Sep 17 00:00:00 2001 From: Keith Hill Date: Sun, 25 Mar 2018 15:58:23 -0600 Subject: [PATCH 3/4] Use test cases and a single Assert. --- .../Session/WorkspaceTests.cs | 46 ++++++++++--------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/test/PowerShellEditorServices.Test/Session/WorkspaceTests.cs b/test/PowerShellEditorServices.Test/Session/WorkspaceTests.cs index 6e8025255..9d5be3d19 100644 --- a/test/PowerShellEditorServices.Test/Session/WorkspaceTests.cs +++ b/test/PowerShellEditorServices.Test/Session/WorkspaceTests.cs @@ -3,12 +3,10 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. // -using Microsoft.PowerShell.EditorServices; using System; using System.IO; -using System.Linq; -using Xunit; using Microsoft.PowerShell.EditorServices.Utility; +using Xunit; namespace Microsoft.PowerShell.EditorServices.Test.Session { @@ -40,30 +38,36 @@ public void CanResolveWorkspaceRelativePath() public void CanDetermineIsPathInMemory() { var tempDir = Environment.GetEnvironmentVariable("TEMP"); - var shortDirPath = Path.Combine(tempDir, "GitHub", "PowerShellEditorServices"); var shortFilePath = Path.Combine(shortDirPath, "foo.ps1"); - var shortDirUriForm = new Uri(shortDirPath).ToString(); - var shortFileUriForm = new Uri(shortFilePath).ToString(); + var shortUriForm = "git:/c%3A/Users/Keith/GitHub/dahlbyk/posh-git/src/PoshGitTypes.ps1?%7B%22path%22%3A%22c%3A%5C%5CUsers%5C%5CKeith%5C%5CGitHub%5C%5Cdahlbyk%5C%5Cposh-git%5C%5Csrc%5C%5CPoshGitTypes.ps1%22%2C%22ref%22%3A%22~%22%7D"; + var longUriForm = "gitlens-git:c%3A%5CUsers%5CKeith%5CGitHub%5Cdahlbyk%5Cposh-git%5Csrc%5CPoshGitTypes%3Ae0022701.ps1?%7B%22fileName%22%3A%22src%2FPoshGitTypes.ps1%22%2C%22repoPath%22%3A%22c%3A%2FUsers%2FKeith%2FGitHub%2Fdahlbyk%2Fposh-git%22%2C%22sha%22%3A%22e0022701fa12e0bc22d0458673d6443c942b974a%22%7D"; - // Test short file absolute paths - Assert.False(Workspace.IsPathInMemory(shortDirPath)); - Assert.False(Workspace.IsPathInMemory(shortFilePath)); - Assert.False(Workspace.IsPathInMemory(shortDirUriForm)); - Assert.False(Workspace.IsPathInMemory(shortFileUriForm)); + var testCases = new[] { + // Test short file absolute paths + new { IsInMemory = false, Path = shortDirPath }, + new { IsInMemory = false, Path = shortFilePath }, + new { IsInMemory = false, Path = new Uri(shortDirPath).ToString() }, + new { IsInMemory = false, Path = new Uri(shortFilePath).ToString() }, - // Test short file relative paths - not sure we'll ever get these but just in case - Assert.False(Workspace.IsPathInMemory("foo.ps1")); - Assert.False(Workspace.IsPathInMemory(".." + Path.DirectorySeparatorChar + "foo.ps1")); + // Test short file relative paths - not sure we'll ever get these but just in case + new { IsInMemory = false, Path = "foo.ps1" }, + new { IsInMemory = false, Path = ".." + Path.DirectorySeparatorChar + "foo.ps1" }, - // Test short non-file paths - Assert.True(Workspace.IsPathInMemory("untitled:untitled-1")); - var shortUriForm = "git:/c%3A/Users/Keith/GitHub/dahlbyk/posh-git/src/PoshGitTypes.ps1?%7B%22path%22%3A%22c%3A%5C%5CUsers%5C%5CKeith%5C%5CGitHub%5C%5Cdahlbyk%5C%5Cposh-git%5C%5Csrc%5C%5CPoshGitTypes.ps1%22%2C%22ref%22%3A%22~%22%7D"; - Assert.True(Workspace.IsPathInMemory(shortUriForm)); + // Test short non-file paths + new { IsInMemory = true, Path = "untitled:untitled-1" }, + new { IsInMemory = false, Path = shortUriForm }, - // Test long non-file path - known to have crashed PSES - var longUriForm = "gitlens-git:c%3A%5CUsers%5CKeith%5CGitHub%5Cdahlbyk%5Cposh-git%5Csrc%5CPoshGitTypes%3Ae0022701.ps1?%7B%22fileName%22%3A%22src%2FPoshGitTypes.ps1%22%2C%22repoPath%22%3A%22c%3A%2FUsers%2FKeith%2FGitHub%2Fdahlbyk%2Fposh-git%22%2C%22sha%22%3A%22e0022701fa12e0bc22d0458673d6443c942b974a%22%7D"; - Assert.True(Workspace.IsPathInMemory(longUriForm)); + // Test long non-file path - known to have crashed PSES + new { IsInMemory = true, Path = longUriForm }, + }; + + foreach (var testCase in testCases) + { + Assert.True( + Workspace.IsPathInMemory(testCase.Path) == testCase.IsInMemory, + $"Testing path {testCase.Path}"); + } } } } From adab98b1e954af745ddb956576edf91d97a53ec3 Mon Sep 17 00:00:00 2001 From: Keith Hill Date: Sun, 25 Mar 2018 15:59:30 -0600 Subject: [PATCH 4/4] Remove test value for verifying we get msg on test fail --- test/PowerShellEditorServices.Test/Session/WorkspaceTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/PowerShellEditorServices.Test/Session/WorkspaceTests.cs b/test/PowerShellEditorServices.Test/Session/WorkspaceTests.cs index 9d5be3d19..41679285f 100644 --- a/test/PowerShellEditorServices.Test/Session/WorkspaceTests.cs +++ b/test/PowerShellEditorServices.Test/Session/WorkspaceTests.cs @@ -56,7 +56,7 @@ public void CanDetermineIsPathInMemory() // Test short non-file paths new { IsInMemory = true, Path = "untitled:untitled-1" }, - new { IsInMemory = false, Path = shortUriForm }, + new { IsInMemory = true, Path = shortUriForm }, // Test long non-file path - known to have crashed PSES new { IsInMemory = true, Path = longUriForm },