Skip to content

Conversation

@BrucePay
Copy link
Collaborator

PR Summary

Fix for #7551. Implemented a new action LocationChangedAction that fires when the current directory in a runspace is changed.

PR Checklist

@BrucePay BrucePay requested a review from daxian-dbw as a code owner August 17, 2018 00:47
Copy link
Collaborator

@iSazonov iSazonov left a comment

Choose a reason for hiding this comment

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

@BrucePay Please address CodeFactor issues for your new code and public comments.

(Get-Variable newPath).Value = $_.newPath
}
Set-Location ..
$newPath.Path | Should Be $pwd.Path
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please use new Pester syntax - Should -Be.
Below too.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oops. Yeah - I'll fix that. Thanks.

Copy link
Collaborator

@rjmholt rjmholt left a comment

Choose a reason for hiding this comment

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

This looks good to me, modulo @iSazonov's Pester comment

Set-Location ..
$ExecutionContext.InvokeCommand.LocationChangedAction = { throw "Boom" }
# Verify that the exception occurred
{ Set-Location $location } | Should Throw "Boom"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ideally this should be Should -Throw -ErrorId "Boom" here

Copy link
Collaborator

@iSazonov iSazonov left a comment

Choose a reason for hiding this comment

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

LGTM with one comment about -Throw.

@anmenaga
Copy link

@TravisEz13 This looks read for merge.
Please note that it has "User-facing Documentation needed".

@TravisEz13 TravisEz13 changed the title Added LocationChangedAction handler + tests to support the Windows Compat module. Added LocationChangedAction handler and tests to support the Windows Compat module. Aug 22, 2018
@TravisEz13 TravisEz13 changed the title Added LocationChangedAction handler and tests to support the Windows Compat module. Add LocationChangedAction handler and tests to support the Windows Compat module. Aug 22, 2018
@TravisEz13 TravisEz13 merged commit 4943bbf into PowerShell:master Aug 22, 2018
@iSazonov iSazonov mentioned this pull request Aug 23, 2018
11 tasks
@TravisEz13 TravisEz13 added this to the v6.1.0 milestone Aug 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants