Powershell: ArgumentCompleter script-block output is always placed unquoted on the command line

[mklement0]

Currently, whatever the script block passed to an ArgumentCompleterAttribute outputs is placed _as-is, unquoted_ on the command line during tab completion.

For simple tokens without embedded whitespace or any other shell metacharacters that works fine, but returning strings such as System.Collections.Generic.List`1 or yes & no results in a malfunctioning or broken command.

The returned string should instead be quoted - either with enclosing single quotes
('System.Collections.Generic.List`1' or 'yes & no') or with character-individual escapes
(System.Collections.Generic.List``1 or yes` `& `no)

Note: Technically, this is a breaking change; code may exist that works around the current problem by using embedded quoting as part of the string returned.

Steps to reproduce

function foo {
  param(
    [ArgumentCompleter( {
        param($cmd, $param, $wordToComplete)
        'a`b', 'a b' -like "*$wordToComplete*"
      })]
    [string] $Bar
  )
}

# Invoke foo
foo a<tab>

Expected behavior

Pressing tab should cycle through placing 'a`b' and 'a b' - note the enclosing quotes - or a``b and a` b on the command line.

Actual behavior

_Unquoted_ a`b and a b are placed on the command line, which would break the command on submission.

Environment data

PowerShell Core 7.0.0-preview.6

[iSazonov]

Maybe the completer should care about quoting?

[mklement0]

That's what you currently have to do, @iSazonov, but the point of the issue is that you shouldn't have to:

Having to do so yourself:

• offers no benefit: there's never a case where you'd want something that is _not_ syntactically valid placed on the command line.

• is therefore only an unnecessary burden that is easy to forget and get wrong (who knows off the top of their head the complete set of all argument-mode metacharacters?).

[iSazonov]

who knows off the top of their head the complete set of all argument-mode metacharacters?

I thought this is implied that completer author does quoting if nessesary because the completer returns a _result_, not a _workpiece_.

[mklement0]

I don't know that you mean by _workpiece_, but there's only one sensible way to use the result returned by the completer: to use it as a syntactically valid argument on the command line being typed.

PowerShell knows what constitutes a syntactically valid argument and can easily transform what the completer script block returns into one.

As I said, there's zero benefit to putting the burden of satisfying external syntax requirements on the author of the script block. All the script block author should need to think about is constructing the _verbatim_ value to return, and let PowerShell handle its quoting, if and as necessary.

[iSazonov]

if and as necessary

My concern is just about this - can PowerShell be always so smart? Will the same completer work right for Path and LiteralPath parameters for example?

[mklement0]

An argument completer is a _resolver_ of sorts: what it returns should always be treated as a _literal_ from a _parsing_ perspective.

If someone writes a completer for an argument that requires parameter-specific _embedded_ escaping - such as the ` chars. you must _embed_ in wildcard expressions to treat metacharacters such as [ as literals, for instance, than that is indeed the script-block author's responsibility - but that doesn't conflict with what I'm proposing here.

[vexx32]

Currently you also have to care about if the user provides a partial string that begins with a quotation mark as well.

Ideally, quoting etc should be completely hidden from a completely; its job is to provide results. It should be the supporting framework making sure that the returned results can actually be used by the command parser.

[bpayette]

@mklement0 Always quoting the result would mean that the only thing you could return is a string. Returning a scriptblock, for example, would not work. Or a cmdlet call e.g. `"(Get-Date)". The completer returns text that is parsed as arbitrary code by PowerShell. This is very powerful.

[lzybkr]

I think it's rare that one wants non-string values when completing parameter values and I think it would be useful to quote automatically.

I provide a helper in TabExpansionPlusPlus for exactly this purpose, see https://github.com/lzybkr/TabExpansionPlusPlus/blob/deea6b96fcfb290f98da44104336f27d0be68fed/TabExpansionPlusPlus.psm1#L64

GitHub

lzybkr/TabExpansionPlusPlus

A V3 PowerShell module to improve tab expansion and Intellisense - lzybkr/TabExpansionPlusPlus

[jhoneill]

I having written dozens of completers myself since @lzybkr provided TabExpansion++ for V3. (later using Register-ArgumentCompleter and then using classes).
Breaking a decade's worth of completers would be a problem, but well written completers DON'T return strings, but CompletionResult object and the example relies on a string being converted to one automatically.
An extra constructor for completion results which took a string and a boolean for "auto quote" would make life easier for people writing completers properly, ([CompletionResult]::new($string) can't change current behavior) but that doesn't solve the specific case here.

Logic which says when converting from a string (and no other type) AND the string isn't already wrapped in quotes AND it contains a space or ' ; # etc it should be wrapped - that would work. I think it is possible to identify classes of strings which would break, and fixing something which pulls up a list of surnames and breaks the command line because author never allowed for O'Neill (to make this personal) might be a good thing.
It would break completers which use a string to provide a set of value, eg select expands to "First", "First, last" ,"First, Last, Address", "First, Age" etc but I don't think that is common. Completers are sufficiently valuable (in my view) that if a change led to more being written, at the cost of breaking this [mis]use case, that might be acceptable.

[mklement0]

Automatic on-demand quoting _unless embedded enclosing quoting is present_ sounds like a good approach to making the behavior useful for the typical use case while breaking only exotic edge cases - which would make this change fall into bucket 3: Unlikely Grey Area.

I think it should similarly be acceptable to change how the _incoming_ token is passed (which @vexx32 mentioned), so that the user having typed something like 'foo', "foo", 'foo or "foo (verbatim) at the time of pressing tab should be passed as just foo to the $wordToComplete parameter.

I don't know enough about the internals, but to accommodate the case where you really want the freedom to receive and return any token _as-is_ (to accommodate @bpayette's example of (Get-Date), for instance), it would make sense to offer a new Boolean _property_, say Raw, on the ArgumentCompleter class (not as a new constructor parameter for CompletionResult), as well as a -RawProcessing switch for Register-ArgumentCompleter.

[SteveL-MSFT]

I hit this in my new module. Pretty annoying that everyone needs to handle quoting. We shouldn't break compatibility by doing it automatically, but I think some helper functions would be sufficient.

[lzybkr]

@SteveL-MSFT - If a completer returns a CompletionResult, I think PowerShell can safely handle quoting automatically because the engine knows the context of the completion, e.g. a parameter value or command name.

If it returns a string, then I agree there are corner cases where you could not safely add quotes automatically.