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.

Sign up for GitHub

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

Fix codelens crash when a file does not exist #732

Merged
merged 4 commits into from
Aug 28, 2018
Merged

Fix codelens crash when a file does not exist #732

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5ac8362
[Style] Refactor FindReferencesOfSymbol code
Aug 23, 2018
700d4f6
Check that file exists before parsing for symbols
Aug 23, 2018
5ba4f30
Fix unneccessary file read for workspace script files
rjmholt Aug 24, 2018
b5ad8cd
Fix unsupported API usage in .NET Framework
rjmholt Aug 28, 2018
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
Prev Previous commit
Next Next commit
Fix unneccessary file read for workspace script files
  • Loading branch information
@rjmholt
rjmholt committed Aug 24, 2018
commit 5ba4f301f674367f00f7db84c4a43289f0ea92ac
14 changes: 7 additions & 7 deletions src/PowerShellEditorServices/Language/LanguageService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using System.Linq;
using System.Management.Automation;
using System.Management.Automation.Language;
using System.Runtime.InteropServices;
using System.Threading;
using System.Threading.Tasks;

Expand Down Expand Up @@ -308,24 +309,23 @@ public async Task<FindReferencesResult> FindReferencesOfSymbol(
await GetAliases();

// We want to look for references first in referenced files, hence we use ordered dictionary
var fileMap = new OrderedDictionary(StringComparer.OrdinalIgnoreCase);
// TODO: File system case-sensitivity is based on filesystem not OS, but OS is a much cheaper heuristic
var fileMap = RuntimeInformation.IsOSPlatform(OSPlatform.Linux)
? new OrderedDictionary()
: new OrderedDictionary(StringComparer.OrdinalIgnoreCase);
foreach (ScriptFile file in referencedFiles)
{
fileMap.Add(file.FilePath, file);
}

IEnumerable<string> allFiles = workspace.EnumeratePSFiles();
foreach (string file in allFiles)
foreach (string file in workspace.EnumeratePSFiles())
{
if (!fileMap.Contains(file))
{
ScriptFile scriptFile;
try
{
scriptFile = new ScriptFile(
file,
clientFilePath: null,
powerShellVersion: this.powerShellContext.LocalPowerShellVersion.Version);
scriptFile = workspace.GetFile(file);
Copy link
Member

Choose a reason for hiding this comment

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

this is the change that will increase perf, yeah @SeeminglyScience?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah that constructor for ScriptFile (which is now unused, but is public, so...) does a call to File.ReadAllText() -- so it always reads off the file system. I'm always averse to something like touching the filesystem in a constructor (and don't really understand why it's done in this constructor, since there's another one that takes a full string), but the real problem is that it's likely we already have the script file contents loaded, and if we don't, this will cache them too

}
catch (IOException)
{
Expand Down