-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Add -Path and -LiteralPath parameters to Test-Json cmdlet
#19042
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
Add -Path and -LiteralPath parameters to Test-Json cmdlet
#19042
Conversation
69168a5 to
ce486a4
Compare
-Path parameter to Test-Json cmdlet
src/Microsoft.PowerShell.Commands.Utility/commands/utility/TestJsonCommand.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.PowerShell.Commands.Utility/commands/utility/TestJsonCommand.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.PowerShell.Commands.Utility/resources/TestJsonCmdletStrings.resx
Outdated
Show resolved
Hide resolved
test/powershell/Modules/Microsoft.PowerShell.Utility/assets/invalid_node.json
Outdated
Show resolved
Hide resolved
src/Microsoft.PowerShell.Commands.Utility/commands/utility/TestJsonCommand.cs
Outdated
Show resolved
Hide resolved
a47799b to
f6b9cf7
Compare
src/Microsoft.PowerShell.Commands.Utility/commands/utility/TestJsonCommand.cs
Outdated
Show resolved
Hide resolved
-Path parameter to Test-Json cmdlet-Path and -LiteralPath parameters to Test-Json cmdlet
17bd510 to
992b89f
Compare
|
@iSazonov Thanks for the feedback. I've included a Let me know what you think, I think it looks much better already 😄. |
8a95fb8 to
9a7582b
Compare
|
One strange thing I've noticed is if you do |
ValueFromPipelineByPropertyName and PSPath alias for LiteralPath work. It is expected behavior. |
src/Microsoft.PowerShell.Commands.Utility/commands/utility/TestJsonCommand.cs
Outdated
Show resolved
Hide resolved
|
Closed and Reopened to restart CI |
d2fbe31 to
707c135
Compare
af3f172 to
1114dfd
Compare
|
This PR has Quantification details
Why proper sizing of changes matters
Optimal pull request sizes drive a better predictable PR flow as they strike a
What can I do to optimize my changes
How to interpret the change counts in git diff output
Was this comment helpful? 👍 :ok_hand: :thumbsdown: (Email) |
daxian-dbw
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks!
|
@ArmaanMcleod Thanks for your contribution! |
|
🎉 Handy links: |
| private const string JsonPathParameterSet = "JsonPath"; | ||
| private const string JsonPathWithSchemaStringParameterSet = "JsonPathWithSchemaString"; | ||
| private const string JsonPathWithSchemaFileParameterSet = "JsonPathWithSchemaFile"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"JsonPath" is really unfortunate naming as it suggests JSON Path, the query syntax.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gregsdennis Agree, we could change parameter set names to something more suitable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please wait until my #18141 merges, or I can do it afterward. That change almost erased these due to a bad rebase.
PR Summary
Fixes #15544
Fixes #12882
Added
-Pathand-LiteralPathparameter toTest-Jsoncmdlet.PR Context
Currently you can only pass a JSON string to
Test-Jsonlike so:With this change you can now also pass a path:
Or with literal path:
Which makes validating JSON from file paths much easier.
Current Syntax
New Syntax
Which does introduce a Bucket 4 breaking change: https://github.com/PowerShell/PowerShell/blob/master/docs/dev-process/breaking-change-contract.md#bucket-4-clearly-non-public
But given changing parameter sets are not technically public API then it seemed fine to reorganize them to allow the new
-Pathparameter. If there is an easier way to do the parameter sets please let me know 😄.PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:or[ WIP ]to the beginning of the title (theWIPbot will keep its status check atPendingwhile the prefix is present) and remove the prefix when the PR is ready.-Pathand-LiteralPathparameter inTest-JsonUtility Cmdlet MicrosoftDocs/PowerShell-Docs#9814(which runs in a different PS Host).