Skip to content

Fix Property and ScriptBlock expressions in EntrySelectedBy tags within custom controls #7913

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

Merged

Conversation

SeeminglyScience
Copy link
Collaborator

@SeeminglyScience SeeminglyScience commented Oct 1, 2018

PR Summary

This change sends the current object to expressions defined in EntrySelectedBy tags within custom controls. Currently attempting to use property expression binding will result in a NullReferenceException, and within a script block expression binding $_/$PSItem will not be set.

Resolves #7847

PR Checklist

Send the current object when evaluating `EntrySelectedBy` in a custom control definition
@iSazonov
Copy link
Collaborator

iSazonov commented Oct 2, 2018

I think we need tests.

@SteveL-MSFT
Copy link
Member

@SeeminglyScience please add tests to Format-Custom.Tests.ps1. You can look at this as an example.

@SeeminglyScience
Copy link
Collaborator Author

@SteveL-MSFT Sorry I saw your reply just a few minutes late. I've added tests, though they are in a different area. I'll go ahead and move them there.

testing
'@ -replace '\r?\n', [Environment]::NewLine

$ps.Invoke()[0].Trim() | Should -BeExactly $expectedOutput
Copy link
Member

Choose a reason for hiding this comment

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

Why index 0 and trim?

Copy link
Collaborator Author

@SeeminglyScience SeeminglyScience Oct 5, 2018

Choose a reason for hiding this comment

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

Index 0 because I know the collection will only have one item. It's mostly just habit from methods that could exist on the collection as well like ToString, I can remove it.

Trim because white space isn't important to the test, and it saves some white space when declaring the expected output. That's just cosmetic and can also be changed.

Copy link
Member

Choose a reason for hiding this comment

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

I understand the habit, but prefer to remove the index. With regard to whitespace, I prefer to explicitly check for it as we had regressions previously with regard to whitespace in the output particularly with tables.

@iSazonov iSazonov self-assigned this Oct 5, 2018
@iSazonov
Copy link
Collaborator

@SteveL-MSFT Please update your review.

Copy link
Member

@SteveL-MSFT SteveL-MSFT left a comment

Choose a reason for hiding this comment

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

LGTM

@iSazonov iSazonov merged commit 680ad9c into PowerShell:master Oct 11, 2018
@iSazonov
Copy link
Collaborator

@SeeminglyScience Thanks for your contribution!

adityapatwardhan pushed a commit to adityapatwardhan/PowerShell that referenced this pull request Apr 9, 2019
…in custom controls (PowerShell#7913)

Send the current object when evaluating `EntrySelectedBy` in a custom control definition
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.

EntrySelectedBy in CustomControls do not pass on the current object to expressions
3 participants