(GH-811) Fix folding with spanning regions

[glennsarti]

From PowerShell/vscode-powershell#1631

Fixes #811

---

In the use case above the following code snippet is confusing the folder;

Bad folder

foreach ($1 in $2) {     <----- STARTS MATCH HERE (1)

    $x = @{  <-----   STARTS MATCH HERE (2)
        'abc' = 'def'
    }        <----- ENDS   MATCH HERE (1) (2)
}

Good folder

foreach ($1 in $2) {     <----- STARTS MATCH HERE (1)

    $x = @{  <-----   STARTS MATCH HERE (2)
        'abc' = 'def'
    }        <----- ENDS  MATCH HERE (2)
}        <----- ENDS  MATCH HERE (1)

The old grammar based folder knew the difference between a } terminating a hash vs a script block. The PowerShell Tokeniser does not.

Note I found this is also the case for @( ... ), ( ... ), and $( ... )

[glennsarti]

@rjmholt @TylerLeonhardt @rkeithhill Although this is WIP, looking for a quick review. I need to cleanup the commit message.

[TylerLeonhardt]

LGTM thanks for jumping this!

[glennsarti]

@TylerLeonhardt This is no longer WIP. Ready for merge.

[TylerLeonhardt]

This LGTM!

[SeeminglyScience]

Changes LGTM, just one small request.

Also, this definitely isn't a blocker for this PR, but looking over the implementation, would something like this make sense instead?

private readonly Stack<Token> _curlyEndStarts = new Stack<Token>();

private readonly Stack<Token> _parenEndStarts = new Stack<Token>();

internal FoldingReference[] FoldableRegions(Tokens[] tokens, bool showLastLine)
{
    var foldableRegions = new List<FoldableReference>();
    foreach (Token token in tokens)
    {
        Token start;
        switch (token.TokenKind)
        {
            case TokenKind.AtCurly:
            case TokenKind.LCurly:
                _curlyEndStarts.Push(token);
                break;
            case TokenKind.RCurly:
                start = _curlyEndStarts.Pop();
                foldableRegions.Add(CreateFoldingReference(start, token.Extent.EndLineNumber));
                break;
            case TokenKind.AtParen:
            case TokenKind.LParen:
                _parenEndStarts.Push(token);
                break;
            case TokenKind.RParen:
                start = _parenEndStarts.Pop();
                foldableRegions.Add(CreateFoldingReference(start, token.Extent.EndLineNumber));
                break;
        }
    }
}

That way you'd only need to enumerate once, it should already be sorted, and no nulls. Sorry if this was already brought up in the original PR

[glennsarti]

@SeeminglyScience Just a note that I can't reply easily to review comments.

That way you'd only need to enumerate once, it should already be sorted, and no nulls. Sorry if this was already brought up in the original PR

• The sorting will not be solved due to the comments foldingreferences being appended later.
• The way it's currently written nulls can still be written if the range doesn't span at least two lines
• While slightly inefficient, multiple passes of the token array is very cheap (sub ms)

Note that PR #806 removes all those code paths anyway.

[rjmholt]

LGTM!

[rjmholt]

This is the edge case I like to see. Allowing these awful variable names is one of @JamesWTruher's proudest additions to PowerShell :)

[glennsarti]

Thanks for adding that