Skip to content

Conversation

TylerLeonhardt
Copy link
Member

fixes PowerShell/vscode-powershell#1689

to make sure we don't hit a NRE here.

The ToLower is suspicious a few lines down... might have to revisit that...

Copy link
Contributor

@rkeithhill rkeithhill left a comment

Choose a reason for hiding this comment

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

LGTM except that ToLower() seems like it should only happen on case-insensitive filesystems. Better yet, the dictionary should be created case-insensitive on those filesystems. Without a change, seems like this could cause bugs on Linux.

@TylerLeonhardt
Copy link
Member Author

Yep I totally agree. I imagine there are more than 1 instances of this... I really need to audit the code on how many times we're not honoring case-sensitive file systems.

Copy link
Collaborator

@SeeminglyScience SeeminglyScience left a comment

Choose a reason for hiding this comment

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

LGTM

Yep I totally agree. I imagine there are more than 1 instances of this... I really need to audit the code on how many times we're not honoring case-sensitive file systems.

Yeah I know for sure it's come up in review at least once or twice before. A sweep for other instances sounds like a great idea.

@TylerLeonhardt TylerLeonhardt merged commit 61e57b3 into PowerShell:master Jan 11, 2019
@TylerLeonhardt TylerLeonhardt deleted the fix-nre-1689 branch January 11, 2019 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants