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.

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

Get-help may return multiple instances of the same help file #3410

Merged
merged 3 commits into from Mar 28, 2017

Conversation

Projects
None yet
5 participants
@SteveL-MSFT
Member

SteveL-MSFT commented Mar 24, 2017

While building directories to search it may add the default shell directory twice to the list of directories to search (this is more of a perf issue of searching it twice than the root cause of the problem)

When a file is found, it generates a new unique key, however, the unique key is based on the root folder and not the actual one that contains the file, so a duplicate can result.

Addresses #3399

Help was incorrectly returning multiple instances of the same help fi…
…le if it existed under a culture path and the parent was in the search path as well
@mirichmo

This comment has been minimized.

Show comment
Hide comment
@mirichmo
Member

mirichmo commented Mar 25, 2017

@SteveL-MSFT

This comment has been minimized.

Show comment
Hide comment
@SteveL-MSFT

SteveL-MSFT Mar 25, 2017

Member

I'll have to debug this on a Mac

Member

SteveL-MSFT commented Mar 25, 2017

I'll have to debug this on a Mac

@SteveL-MSFT

This comment has been minimized.

Show comment
Hide comment
@SteveL-MSFT

SteveL-MSFT Mar 27, 2017

Member

@iSazonov you ready to approve the changes?

Member

SteveL-MSFT commented Mar 27, 2017

@iSazonov you ready to approve the changes?

@iSazonov

This comment has been minimized.

Show comment
Hide comment
@iSazonov

iSazonov Mar 28, 2017

Collaborator

@SteveL-MSFT I cannot repo the Issue locally and don't still understand a root of the Issue. I run the test in PowerShell Core without the fix and the test doesn't fail. 😕

Collaborator

iSazonov commented Mar 28, 2017

@SteveL-MSFT I cannot repo the Issue locally and don't still understand a root of the Issue. I run the test in PowerShell Core without the fix and the test doesn't fail. 😕

@SteveL-MSFT

This comment has been minimized.

Show comment
Hide comment
@SteveL-MSFT

SteveL-MSFT Mar 28, 2017

Member

@iSazonov did you run on Linux? It doesn't repro on Windows

Member

SteveL-MSFT commented Mar 28, 2017

@iSazonov did you run on Linux? It doesn't repro on Windows

@iSazonov

This comment has been minimized.

Show comment
Hide comment
@iSazonov

iSazonov Mar 28, 2017

Collaborator

Oh, I only test on Windows. Sorry, I do not have the opportunity to build on Unix.

Collaborator

iSazonov commented Mar 28, 2017

Oh, I only test on Windows. Sorry, I do not have the opportunity to build on Unix.

@Francisco-Gamino

This comment has been minimized.

Show comment
Hide comment
@Francisco-Gamino

Francisco-Gamino Mar 28, 2017

Contributor

@SteveL-MSFT: The change LGTM. Thanks Steve for making this change!

Contributor

Francisco-Gamino commented Mar 28, 2017

@SteveL-MSFT: The change LGTM. Thanks Steve for making this change!

@SteveL-MSFT

This comment has been minimized.

Show comment
Hide comment
@SteveL-MSFT

SteveL-MSFT Mar 28, 2017

Member

@Francisco-Gamino can you mark Approved in the GitHub review tool?

Member

SteveL-MSFT commented Mar 28, 2017

@Francisco-Gamino can you mark Approved in the GitHub review tool?

@mirichmo mirichmo merged commit da2fd5c into PowerShell:master Mar 28, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@SteveL-MSFT SteveL-MSFT deleted the SteveL-MSFT:help branch Jun 28, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment