Skip to content
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

Network share path expansion should support forward slashes #17111

Closed
sba923 opened this issue Apr 5, 2022 · 11 comments · Fixed by #17117
Closed

Network share path expansion should support forward slashes #17111

sba923 opened this issue Apr 5, 2022 · 11 comments · Fixed by #17117
Labels
Issue-Enhancement the issue is more of a feature request than a bug Resolution-Fixed The issue is fixed.

Comments

@sba923
Copy link
Contributor

sba923 commented Apr 5, 2022

Summary of the new feature / enhancement

As a user using the French keyboard layout, entering a forward slash (Shift+:) is way more convenient than entering a backslash (AltGr+8). So using forward slashes throughout PowerShell has become muscle memory...

This works in most cases.

Alas, tab expansion for inputs like

\\hostname\x<TAB>

works, but this doesn't:

//hostname/x<TAB>

Proposed technical implementation details (optional)

No response

@sba923 sba923 added Issue-Enhancement the issue is more of a feature request than a bug Needs-Triage The issue is new and needs to be triaged by a work group. labels Apr 5, 2022
@MartinGC94
Copy link
Contributor

MartinGC94 commented Apr 5, 2022

This is handled here: https://github.com/PowerShell/PowerShell/blob/master/src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs#L4304 at a glance, it looks like a relatively simple thing to fix.

GitHub
PowerShell for every system! Contribute to PowerShell/PowerShell development by creating an account on GitHub.

@sba923
Copy link
Contributor Author

sba923 commented Apr 5, 2022

Had almost found it 😆, now I need to remember how to build the repo and will give it a shot!

@sba923
Copy link
Contributor Author

sba923 commented Apr 5, 2022

Tomorrow is another day 😜

image

@SeeminglyScience
Copy link
Collaborator

@sba923 You just wanna add -ResGen

@sba923
Copy link
Contributor Author

sba923 commented Apr 5, 2022

@sba923 You just wanna add -ResGen

Thx.

Build still emits this error... after producing pwsh.exe:

image

With this change, the expected result seems to be achieved (though the completion turns the forward slashes into backslashes, but that's OK):

image

Is that good enough or would you consider optimizing the regex?

@sba923
Copy link
Contributor Author

sba923 commented Apr 6, 2022

Is adding this comment OK? What's the preferred way of referring to issues? Absolute GH URL or #nnnnn?

// support both / and \ when entering UNC paths for typing convenience (#17111)

@SeeminglyScience
Copy link
Collaborator

SeeminglyScience commented Apr 6, 2022

Build still emits this error... after producing pwsh.exe:

Yeah I dunno what's up with that but I get it too so 🤷

Is that good enough or would you consider optimizing the regex?

That can be settled in the review. I would recommend making it a literal (@"pattern here") instead though so it's somewhat feasible to read 😁 (I know it was like that before, but while we're there might as well).

Is adding this comment OK? What's the preferred way of referring to issues? Absolute GH URL or #nnnnn?

I think that's good, but also can be settled in the review if needed.

@sba923
Copy link
Contributor Author

sba923 commented Apr 6, 2022

OK. This works fine, so I'll proceed with creating the PR:

image

@sba923
Copy link
Contributor Author

sba923 commented Apr 6, 2022

PR #17117 created. Was not completely sure about the "impacts completers" section of the checklist, though.

sba923 added a commit to sba923/PowerShell that referenced this issue Apr 9, 2022
sba923 added a commit to sba923/PowerShell that referenced this issue Apr 10, 2022
@ghost ghost added the In-PR Indicates that a PR is out for the issue label Apr 12, 2022
@ghost ghost added Resolution-Fixed The issue is fixed. and removed Needs-Triage The issue is new and needs to be triaged by a work group. In-PR Indicates that a PR is out for the issue labels Apr 12, 2022
@ghost
Copy link

ghost commented May 23, 2022

🎉This issue was addressed in #17117, which has now been successfully released as v7.3.0-preview.4.:tada:

Handy links:

@ghost
Copy link

ghost commented May 23, 2022

🎉v7.3.0-preview.4 has been released which incorporates this pull request.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Enhancement the issue is more of a feature request than a bug Resolution-Fixed The issue is fixed.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants