This is purely just to show the work I've done so far. It is far from complete (and breaks about 10,000 typescript linting rules :-) )

My current thinking is;

• Use the client side textmate grammar file to tokenize the source
• Once I have that, I can make decisions as to where code begins
• Once I know where code begins and ends I can craft the required FoldingRange objects

Note that the syntax folding supersedes any "default" folding regions e.g. # region or line break/indentation style.

I have brackets ( { } and ( ) ) working

brackets

It's hard to see but...you'll notice the default folding is there until the extension is loaded, which "magically" removes the folding for the # region section.

Use the client side textmate grammar file to tokenize the source
Once I have that, I can make decisions as to where code begins

It sounds like you are redoing work the parser has already done. For example, getting here string folding region info amounts to this:

$tok = $err = $null
$ast = [System.Management.Automation.Language.Parser]::ParseFile("$pwd\examples\DebugTest.ps1", [ref]$tok, [ref]$err)
$ast.FindAll({param($ast) ($ast -is [System.Management.Automation.Language.StringConstantExpressionAst]) -and (($ast.StringConstantType -eq [System.Management.Automation.Language.StringConstantType]::DoubleQuotedHereString) -or ($ast.StringConstantType -eq [System.Management.Automation.Language.StringConstantType]::SingleQuotedHereString))}, $true) | 
 Foreach-Object Extent |
 Select-Object StartLineNumber, EndLineNumber

Which, for my test file outputs:

StartLineNumber EndLineNumber
--------------- -------------
 3 9
 34 36

We have the ability from the TS extension source to make a request over to the PS Editor Services such that it can execute PowerShell code for us including queries on the AST. This is how the code analysis diagnostics are reported. This is how help completion works. This is how "Find PS modules from Gallery" works. The one area that the AST doesn't help with is comments which includes region support - since that is implemented as "special" comment. Anyway, I had envisioned the folding support would take advantage of the AST for most everything else.

If you need help with the infrastructure to callback to PSES as well as the PSES C# infrastructure to process the request - let me know.

Hi @rkeithhill . Thanks for taking the time to look at my WIP. I did consider using the Language Server to surface this information but I decided against it for the following reasons:

1. If the language server crashes, you lose folding. This is a "breaking change" behaviour of the extension, where folding still works (Indentation based) if the lang. server is unavailable

2. VS Code is the only editor I can see that allows syntax aware folding (Atom and Sublime do not. Didn't check VIM). So this feature is specific to VSCode so it seems to belong in the vscode extension, not the generic Editor Services.

3. You can get the region comments by using the tokens generated by the ::ParseFile method ($tok). But at this point you're replicating things in the lang. server you could do client side.

4. Reduce complexity. By adding another "thing" to the lang. server. including all the work to add that "thing", and all the plumbing that goes with it, is the increased complexity (and to some degree latency) worth it?. In my opinion, no. We can do the same work client side (using the grammar file), with almost exact accuracy as the native PowerShell parser, without the complexity overhead.

Note - By complexity, I'm more referring to the system as a whole, as opposed to complex for a developer to write/understand.

5. (This point is debatable) I feel it makes more sense for the token decisions to come from the textmate grammar than the lang. server.

Folding should follow the visual cues of the editor. The visual cues come from the colourisation and syntax highlighting, which come the textmate grammar. These cues do not come from the lang. server. But ... you could easily argue that the lang. server knows more about the file and should be considered the authority.

Of course there are downsides to going client side:

1. The powershell grammar tokens I'm looking for would become hard-coded. So if there were changes to the scope names in the grammar file, the folding provider would fail to see them. This means the client side method is fragile to grammar scope name changes.

Now, is this possible? of course it's possible we'll have changes. Is this probable? For the tokens I'm looking for, not likely. Is this testable? Yes, tests could be written to make sure the folding provider continues to function as expected.

If the language server crashes, you lose folding.

And intellisense, code analysis, code formatting, code lens, etc. When the language server crashes, it's a bad day. I don't think that's a good reason to not utilize the language server for some of this.

you could easily argue that the lang. server knows more about the file and should be considered the authority.

Yup. In addition, the tm grammar we've been using has been a steady source of issues/complaints. I'm not particularly confident in its ability to support PowerShell adequately. This bug was fixed recently (the = after ParameterSetName throws it off):

image

For the longest time, Where-Object, Sort-Object, Select-Object wouldn't get colorized correctly. That was recently fixed but we still get a pretty steady stream of tm grammar issues - see https://github.com/powershell/editorSyntax/issues

I'm probably more than a little jaded about the tm grammar. :-) Heck, if I had my druthers we'd use an API like what is being proposed here: microsoft/vscode#585 Which is essentially how Visual Studio does syntax colorization. It asks the language service to provide colorization info.

By adding another "thing" to the lang. server. including all the work to add that "thing", and all the plumbing that goes with it, is the increased complexity (and to some degree latency) worth it?

Depends. If you can provide all the desired folding regions without all the corner cases bugs we seem to so frequently run into with the tm grammar, then no, it's not worth using the AST via the lang server. OTOH I'd have more confidence in the information we get from AST i.e. I wouldn't worry about corner-cases. But I'm willing to be proven wrong. It could be my low opinion of the tm grammar is unwarranted. :-)

BTW, lest I come off too negative, I really do appreciate the contribution!! Proper here string folding is something folks have been asking for, for a long time now. Just want to make sure we have a healthy debate about the best approach and wanted to make you're aware that using the AST is a fairly easy option.

Just want to make sure we have a healthy debate about the best approach

Agreed! FWIW I'm one of the authors of the Puppet extension and I have the same debates where to stick features client vs Lang. Server. Always trade-offs

There is also a middle ground, which is to implement client side until the server side code is complete, and then just switch over. Could even hide it behind a vscode setting to make it an opt-in feature. Adding a command to the language server is not trivial, to me anyway. I am not usually a C# developer so the ramp-up time is not small. The plumbing to call the server from the client I can handle ok (copy-n-paste to a degree). If it's decided to go down that route, not client side, I probably won't be able to assist.

Regarding the TM issues... I personally haven't had them, however I too am just a sample size of one :-)

If you can provide all the desired folding regions without all the corner cases bugs we seem to so frequently run into

For the tokens we're interested in in this PR, I don't feel it's too bad, but that's just a gut feel, no data to back it up.

So to wrap up.

• The Language Server is probably the final resting place for the folding logic to be determined, and right now I will not be able to assist with that.

• But I, as a community member, can provide an interim solution using the grammar file for tokenisation, until the more correct solution (lang server) is implemented. The interim solution can be opt-in which should help with supporting the interim feature.

• Switching over from the client side to server side logic will be transparent to the vscode users.

Sounds good. If I can carve off some time to work on the PSES impl, what info does the client need besides start/end line pairs?

Needs enough information to populate - https://code.visualstudio.com/docs/extensionAPI/vscode-api#FoldingRange

• Start
• End
• Type of region - Comment, Imports (N/A to us) and Region.

Interesting. So what FoldingRangeKind do you use for functions and here strings?

I was going to use 'Region' for everything except comments

I wonder if we should provide feedback to the VSCode team that we'd like to have a few more "kinds" e.g. function/method, verbatim/here string, or maybe just "Other" as a catchall.

Sure. I hadn't thought that far ahead.

Awesome discussion guys :) I agree that PSES should be, in the end, where the folds come from. Another benefit I don't think was mentioned is that other editors could implement syntax aware folding and those editors can get it for free/minimal effort :)

It would also be good to have all the using statements in PowerShell folded as an imports type but this is much lower pri IMO.

Hmmmm I definitely see the motivation to make folding all happen client side. Especially since folding should be simple.

But the truth is that trying to do it that way we will inevitably come across things that a TM Grammar is simply not powerful enough to recognise - PowerShell is woefully grammatically complex. The parser is the final arbiter of what text is what token, and if we handle ASTs properly (i.e. cache them, etc), it should all be very cheap too.

@rkeithhill as an aside: I too really want that hook-in highlighting functionality - would be very useful for things like dynamic keyword support and function highlighting.

@rkeithhill and @rjmholt PR is done. Ready for review.

@omniomi is the big contributor to EditorSyntax which is where the syntax highlighting comes from.

Nick, if you have some time, can you take a look at this PR and give your thoughts on this since this change depends on EditorSyntax?

From a quick look over none of the scopes used for folding in this PR are likely to change as they were all corrected in an older PR. That said, @glennsarti I'll add begin and end scopes to here-strings which I just noticed are missing.

@tylerl0706 I think we're at the point we should create a dependency document for EditorSyntax to track the potential fallout from changes. Ie, where it's used, any specific scope dependencies, as well as processes for having dependents update to newer revisions.

Yeah @omniomi , one thing that's missing from this PR is tests. In theory I should be able to write some simple integration tests (e.g. Given text content X, it should return Y folding regions).

For better or worse, as this is behind a feature flag, I don't need as good test coverage, yet!. This should give me enough time to figure out how to download a grammar test fixture, and write appropriate tests.

@tylerl0706 @rjmholt Addressed all comments. CI is green...