I think this should be public sealed class.

yes

I normally don't see List size initialization unless the size is known. I am curious why you initialize the size to this value?

It is cheaper to allocate a list that is a little larger than to reallocate and copy when out-grown.
But the size can always be argued.

When I profile memory allocations, Array allocations on List.Resize is not uncommon, and when I see them, I try to find a sensible default size do get rid of the most common once.

Join-Object implementation

oops :)

I think you can just check count once and early out or just skip the if/else processing.

Yes, much better!

This doesn't look right to me. This class can own PowerShell/Runspace only if PowerShell is created with Runspace.NewRunspace mode. Otherwise you are disposing a runspace created by someone else (and possibly the thread default runspace).

Good catch!

Reverting the whole file.

Can it be static?

Then you end up with races if used from multiple runspaces.

The list looks as a const for current session. Do I skip something?

Yes, it is captured by the local function AddMatching.
So the result is depending on wordToComplete.

I see now. :-) It is Friday. I can not catch up with my idea :-)
I am looking at 'new CompletionResult()'. If they isn't modifed later we could make it static and only reference in target List?

Can we use PSPropertyExpression type like in #6934?

PropertyExpression has some extra semantics, handling hashtables like @{n="x";ex={2,3,4}}.

I looked at it, but was not convinced it was a fit. But there's definitely an overlap. Input here is most welcome.

Seems the switch is superfluous. We could leave only
``'c#
return Environment.NewLine.Replace("\r", "r").Replace("\n", "n");

Yes it could. I wrote it the way I did to reduce allocated objects. May not be worth it.
The string literals will be loaded from metadata and interned. There will only exist one instance of them. The default line will create two new strings each time.

Environment.NewLine is static. So I think we should use the usual method

public static string NewLineText
{
    get
    {
#if UNIX
        return "`n"
#else
        return "`r`n"
#endif
    }
}

Good point!

But I think MacOS uses `r

No, only very old MacOs versions. See my comment below.

Yes, it is captured by the local function AddMatching.
So the result is depending on wordToComplete.

I'd expand the comment with what is the format and/or add a link on docs.

Please use file pattern:

/// <Summary>
/// A bigger default to not get re-allocations in common use cases.
/// </Summary> 

We could use

_outputBuilder.Append(_quoteChar).Append(stringValue).(_quoteChar); 

I'd want to get confirmation that this should be CurrentCulture not InvariantCulture.
And maybe discuss -FormatCulture parameter.

Same here. Now sure what is right here.

PowerShell consistently uses the invariant culture in to/from-string conversions, so it should probably be InvariantCulture, at least by default, perhaps with an optional -UseCulture switch (analogous to the *-Csv cmdlets).

As a guideline, we use InvariantCulture when manipulating data so that the behaviour of the code is predictable (invariant) across locales and we use current culture when presenting to the user. This cmdlet could be used in both scenarios but I expect it will be used more for data processing so I suggest defaulting to InvariantCulture with an option to override for current culture.

Adding UseCulture switch as suggested.

 gcm -ParameterName Culture

CommandType     Name                                               Version    Source
-----------     ----                                               -------    ------
Cmdlet          Compare-Object                                     3.1.0.0    Microsoft.PowerShell.Utility
Cmdlet          Group-Object                                       3.1.0.0    Microsoft.PowerShell.Utility
Cmdlet          New-PSSessionOption                                6.0.0.0    Microsoft.PowerShell.Core
Cmdlet          Sort-Object                                        3.1.0.0    Microsoft.PowerShell.Utility

We already have -Culture parameter. Make sense re-use the name?
Or -UseCurrentCulture? Looks as limited option.

@iSazonov:

I think -UseCulture is the correct name, after all:

• The -Culture <string> parameters you reference:

  • require an argument, namely a given culture
  • are used with cmdlets that are current-culture-sensitive by default, to allow opt-in to a different culture.

• The -UseCulture switch, by contrast:

  • is used with cmdlets that are culture-invariant by default, to allow opt-in to the current culture (currently applies only to the *-Csv cmdlets).

The latter is what is being implemented here.

@mklement0 Thanks! We need discuss Select-String too.

Please address the comment.

We could remove the test - any test below do the test.