Skip to content

Convert -ChildPath parameter to string[] for Join-Path cmdlet - #24677

Merged
Ilya (iSazonov) merged 5 commits into
PowerShell:masterfrom
ArmaanMcleod:join-path-childpath-parameter-array
Mar 18, 2025
Merged

Convert -ChildPath parameter to string[] for Join-Path cmdlet#24677
Ilya (iSazonov) merged 5 commits into
PowerShell:masterfrom
ArmaanMcleod:join-path-childpath-parameter-array

Conversation

@ArmaanMcleod

@ArmaanMcleod Armaan Mcleod (ArmaanMcleod) commented Dec 15, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

PR Summary

Converted -ChildPath parameter to string[] for Join-Path cmdlet.

PR Context

Fixes #21367

Allows user to give an array of child paths with -ChildPath. This avoids the extra usage with -AdditionalChildPath which can be ignored and makes usage of Join-Path easier.

This was discussed to be bucket 3 breaking change.

Example

> Join-Path -Path 'one' -ChildPath 'two', 'three'
one\two\three

PR Checklist

@ArmaanMcleod Armaan Mcleod (ArmaanMcleod) mentioned this pull request Dec 15, 2024
Closed
2 tasks
@microsoft-github-policy-service microsoft-github-policy-service Bot added the Review - Needed The PR is being reviewed label Dec 22, 2024
@iSazonov

Ilya (iSazonov) commented Mar 10, 2025

Copy link
Copy Markdown
Collaborator

Armaan Mcleod (@ArmaanMcleod) Since it is a breaking change I suggest to wrap this in an experimental feature (you can find examples in our code). Then I can review and merge since it is already approved by WG.

@microsoft-github-policy-service microsoft-github-policy-service Bot removed the Review - Needed The PR is being reviewed label Mar 10, 2025
@ArmaanMcleod

Armaan Mcleod (ArmaanMcleod) commented Mar 10, 2025

Copy link
Copy Markdown
Contributor Author

Ilya (@iSazonov) Interesting. I thought since it was accepted as bucket 3 breaking change it would not need an experimental feature. I did not see that requirement in the issue.

In any case would be good to move this along so I will create an experimental feature for this.

What should the experimental feature be called?

@iSazonov

Ilya (iSazonov) commented Mar 11, 2025

Copy link
Copy Markdown
Collaborator

Although it's a bracket 3, it's still a breaking change. The experimental feature allows us to minimize risks and get feedback faster.
Name could be PSJoinPathChildArray. You can see examples with Get-ExperimentalFeature cndlet.

@ArmaanMcleod

Armaan Mcleod (ArmaanMcleod) commented Mar 15, 2025

Copy link
Copy Markdown
Contributor Author

Ilya (@iSazonov) This one actually doesn't make much sense to enable experimental feature, since we are changing the type of ChildPath from string to string[]. We can't have a parameter type change be wrapped in experimental feature.

@iSazonov

Ilya (iSazonov) commented Mar 15, 2025 •
edited by unfurl-links Bot
Loading

Copy link
Copy Markdown
Collaborator

We can't have a parameter type change be wrapped in experimental feature.

We have ExperimentalAttribute. See https://github.com/daxian-dbw/PowerShell/blob/d12230f1888ea0226df0e4e0e2e0de2f9e8c0a1e/test/powershell/engine/ExperimentalFeature/assets/ExpTest/ExpTest.cs

GitHub
PowerShell for every system. Contribute to daxian-dbw/PowerShell development by creating an account on GitHub.

@ArmaanMcleod

Armaan Mcleod (ArmaanMcleod) commented Mar 15, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Ilya (@iSazonov) Thats really interesting. However even if you do:

[Experimental(ExperimentalFeature.PSJoinPathChildArray, ExperimentAction.Show)]
public string[] ChildPath { get; set; }

How do you also make the previous parameter:

public string ChildPath { get; set; }

Also show when the experimental feature is disabled? in C# we can't have two properties have the same name but different type. Maybe I am misunderstanding how this works...

@iSazonov Ilya (iSazonov) added CL-BreakingChange Indicates that a PR should be marked as a breaking change in the Change Log CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log PowerShell-Docs needed The PR was reviewed and a PowerShell Docs update is needed labels Mar 15, 2025
@iSazonov

Ilya (iSazonov) commented Mar 15, 2025

Copy link
Copy Markdown
Collaborator

in C# we can't have two properties have the same name but different type.

Yes, it would be too tricky wrap in experimental feature. Let's leave as is.

iSazonov
Ilya (iSazonov) reviewed Mar 15, 2025
View reviewed changes
Comment thread test/powershell/Modules/Microsoft.PowerShell.Management/Join-Path.Tests.ps1
}
@{
Path = 'one'
ChildPath = [string]::Empty

@iSazonov Ilya (iSazonov) Mar 15, 2025

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.

We need one more test for empty collection.

@ArmaanMcleod Armaan Mcleod (ArmaanMcleod) Mar 15, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This should already be covered here:

PowerShell/test/powershell/Modules/Microsoft.PowerShell.Management/Join-Path.Tests.ps1

Lines 67 to 71 in 0efc23f

@{
Path = 'one'
ChildPath = @()
ExpectedResult = "one${sepChar}"
}

@iSazonov Ilya (iSazonov) self-assigned this Mar 15, 2025
@iSazonov

This comment was marked as outdated.

Sign in to view
@azure-pipelines

This comment was marked as outdated.

Sign in to view
@iSazonov
Ilya (iSazonov) merged commit 48c34fc into PowerShell:master Mar 18, 2025
@microsoft-github-policy-service

microsoft-github-policy-service Bot commented Mar 18, 2025 •
edited by unfurl-links Bot
Loading

Copy link
Copy Markdown
Contributor

📣 Hey Armaan Mcleod (@ArmaanMcleod), how did we do? We would love to hear your feedback with the link below! 🗣️

🔗 https://aka.ms/PSRepoFeedback

@mdanish-kh Muhammad Danish (mdanish-kh) mentioned this pull request Apr 13, 2025
Merged
SIRMARGIN pushed a commit to SIRMARGIN/PowerShell that referenced this pull request Dec 12, 2025
@ArmaanMcleod @SIRMARGIN
Convert -ChildPath parameter to string[] for Join-Path cmdlet (P…
212fe92 
…owerShell#24677)
Closed
14 tasks
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj/PowerShell that referenced this pull request Aug 6, 2026
@ArmaanMcleod
Convert -ChildPath parameter to string[] for Join-Path cmdlet (P…
c627fd7 
…owerShell#24677)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iSazonov Ilya (iSazonov) iSazonov approved these changes
@daxian-dbw Dongbo Wang (daxian-dbw) Awaiting requested review from daxian-dbw
@adityapatwardhan Aditya Patwardhan (adityapatwardhan) Awaiting requested review from adityapatwardhan
@SteveL-MSFT Steve Lee (SteveL-MSFT) Awaiting requested review from SteveL-MSFT

Assignees

@iSazonov Ilya (iSazonov)

Labels

CL-BreakingChange Indicates that a PR should be marked as a breaking change in the Change Log CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log PowerShell-Docs needed The PR was reviewed and a PowerShell Docs update is needed

Projects

PowerShell Team Reviews/Investigations
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Make Join-Path parameter -ChildPath accept an array

2 participants

@ArmaanMcleod @iSazonov