Skip to content

Add LocationChangedAction handler and tests to support the Windows Compat module. - #7552

Merged
Travis Plunk (TravisEz13) merged 3 commits into
PowerShell:masterfrom
BrucePay:brucepay_ChdirEvent
Aug 22, 2018
Merged

Add LocationChangedAction handler and tests to support the Windows Compat module.#7552
Travis Plunk (TravisEz13) merged 3 commits into
PowerShell:masterfrom
BrucePay:brucepay_ChdirEvent

Conversation

@BrucePay

Copy link
Copy Markdown
Collaborator

PR Summary

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

PR Checklist

@iSazonov Ilya (iSazonov) left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Bruce Payette (@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
Copy Markdown
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
Copy Markdown
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.

@rjmholt Rob Holt (rjmholt) left a comment

Copy link
Copy Markdown
Collaborator

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 Ilya (@iSazonov)'s Pester comment

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

Copy link
Copy Markdown
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

@iSazonov Ilya (iSazonov) left a comment

Copy link
Copy Markdown
Collaborator

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
Copy Markdown

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

@TravisEz13 Travis Plunk (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 Travis Plunk (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
Travis Plunk (TravisEz13) merged commit 4943bbf into PowerShell:master Aug 22, 2018
@iSazonov Ilya (iSazonov) mentioned this pull request Aug 23, 2018
11 tasks
@TravisEz13 Travis Plunk (TravisEz13) added this to the v6.1.0 milestone Aug 23, 2018
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request Aug 6, 2026
…Compat module (PowerShell#7552)

Add LocationChangedAction handler and tests to support the Windows Compat module.
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