@iSazonov I think we can safely remove this code:1bd9919

Escape globbing path chars is not being used anywhere so it really is dead code. Let me know if you you are ok with repurposing this PR.

I do not have a complete understanding of what needs to be escaped and whether this is always a general rule. Or since we are talking about a common method, the question is whether there can be completers for which we need to escape a different set of characters.

I'd want to get a feedback from@MartinGC94.

I agree that the current name is bad, though I'd remove "Path" from the new name as this is not exclusive to paths. However, I don't see much value in renaming a parameter that is unused. Why not just rename it when you start using it for the first time? Or remove it if there's no plans to use it?

As for the code itself, it is flawed and I question its usefulness even if it was fixed.
1: It doesn't check for every problematic character. If I for example create a function like this:New-Item -Path 'function:\-G' -Value {'Hello'} and try to complete it:Get-Command -Name <Tab> it won't be quoted in the list of results because it doesn't check for the dash.
2: Even for the characters it does check it's not useful to quote something just because it has wildcard characters. For example if I created this function:New-Item -Path 'function:\G[a-z]' -Value {'Hello'} it would return true (that quotes are needed) but quoting doesn't stop it from being interpreted as a wildcard so what use is that?
3: Parsing the completion value on its own (meaning it's parsed in Expression Mode) to determine whether or not quotes are needed is not useful when it's an argument because the rules are slightly different. For example an IP address like:192.168.1.1 is invalid in Expression mode as it sees the first 2 octets as a number, and the last 2 octets as accessing a member of that first number. Whereas as an argument:Get-NetIPAddress -IPAddress 192.168.1.1 it's perfectly valid.

@MartinGC94 Thanks!

A comment for the code says about paths and variables. I guess the code was used for file paths and variables many years ago. Later specific code was created for file paths and variables.
So I think we should remove the old code and parameter.

Looking as CompletionRequiresQuotes is used (14 call sites) I see it is forarguments only. So questions are:

1. Currently we check only$ and backtick. Does it make sense to check the chars? What chars are it makes sense to check for follow argument quoting?
2. I see globbing for one char works in tab completionNew-Item -Path c:\W[i]<Tab>. But all globbing features don't work. Is it by design or accidentally? Should we support globbing in tab completion for arguments? I guess it is only a side effect of WildcardPattern using.
3. Should we continue to use Parser for arguments? And does we use it in right way?

@ArmaanMcleod@MartinGC94 What do you think?

So I think we should remove the old code and parameter.

@iSazonov I personally think if we have old code which is not used and doesn't provide much value, we should remove it and perhaps revisit how it is done.

Currently we check only $ and backtick. Does it make sense to check the chars? What chars are it makes sense to check for follow argument quoting?

I think we should check any char which causes problems for escaping single/double quotes. Like I understand the basic examples which need escaping:

• single quotes in single quote e.g.' another, 'quote'
• backticks in double quotes e.g."another, `backtick"

I like how@MartinGC94 has done this with bad chars inhttps://github.com/MartinGC94/UsefulArgumentCompleters/blob/bd94bb0b5fadc92e95ea32f70b22eeb35ab4b0b7/Classes/CompletionHelper.psm1#L21-L46. I am wondering if we can use a similar list for this repo so we can at least reduce all the obvious bad cases which come up. I guess I'm just not sure how to expose all these bad cases.

Should we continue to use Parser for arguments? And does we use it in right way?

I guess my understanding is that when you parse input for tokens you can get errors, so it was an easy way to check a string is not valid and requires quotes.

It's hard to check if it's the right way or if there were suggested alternatives, a lot of theCompletionCompleters.cs GIT history is from the60b3b30 when the repository was opensourced 😄.

GitHub

UsefulArgumentCompleters/Classes/CompletionHelper.psm1 at bd94bb0b5fadc92e95ea32f70b22eeb35ab4b0b7 · MartinGC94/UsefulArgumentCompleters

Contribute to MartinGC94/UsefulArgumentCompleters development by creating an account on GitHub.

@ArmaanMcleod Thanks for referencing the module!

If it is possible to create a test (I mainly assume that we should scenarios where this can happen) for every checked char I believe we can accept the checks from@MartinGC94 module. The same for Parser checks. It is for follow PRs.

As for removing old code, we can continue in the PR.

@iSazonov Thanks. I have repurposed this PR to just remove theescape parameter.

@MartinGC94 Do you have any interest in transferring the escaping capabilities of your module here in the form of several PRs?

My module doesn't really do any escaping. It just adds quotes if needed just like the current code (though obviously I handle a few more special characters in my checks). Any new code for this should ideally also take escaping of globbing characters into account, and it sounds like@ArmaanMcleod is already working on that.

@MartinGC94

My module doesn't really do any escaping.

I was referring to quoting, although escaping may also be necessary.

it sounds like@ArmaanMcleod is already working on that.

He's working on separating the shared code, which we can make public. This is very similar to how your module works.
But neither he nor I have a complete understanding of handling special characters correctly as it is done in your module. Your cooperation would be helpful. If you have the interest and time, it would be great to get a PR with tests and scenario descriptions.

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

🔗https://aka.ms/PSRepoFeedback

Microsoft Forms