Skip to content

feat: select the folding provider to use#157434

Merged
aeschli merged 8 commits into
microsoft:mainfrom
daiyam:custom-folding-strategy
Oct 23, 2022
Merged

feat: select the folding provider to use#157434
aeschli merged 8 commits into
microsoft:mainfrom
daiyam:custom-folding-strategy

Conversation

@daiyam

@daiyam daiyam commented Aug 7, 2022

Copy link
Copy Markdown
Contributor

The PR allows to select the folding provider to use. The name of the provider is the name of the extension that provides it.

It follows the recommendations of @aeschli (#54200 (comment))

ProKashif
ProKashif previously approved these changes Aug 7, 2022
@gjsjohnmurray

Copy link
Copy Markdown
Contributor

@aeschli will this help #138093?

@daiyam

daiyam commented Aug 7, 2022

Copy link
Copy Markdown
Contributor Author

@gjsjohnmurray Yes, it should help in conjunction of an extension like mine (Explicit Folding).

@daiyam

daiyam commented Sep 4, 2022

Copy link
Copy Markdown
Contributor Author

@aeschli Hi, could you tell me if the PR needs any modifications? Thx

@daiyam

daiyam commented Sep 19, 2022

Copy link
Copy Markdown
Contributor Author

+1

@daiyam

daiyam commented Sep 26, 2022

Copy link
Copy Markdown
Contributor Author

@aeschli Hi, what can I do for the PR to be merged? Thx

@aeschli

aeschli commented Sep 26, 2022

Copy link
Copy Markdown
Contributor

Sorry, I didn't find time yet to review the PR...

@aeschli aeschli added this to the October 2022 milestone Oct 22, 2022
@aeschli aeschli added the editor-folding Editor code folding issues label Oct 22, 2022
@aeschli

aeschli commented Oct 22, 2022

Copy link
Copy Markdown
Contributor

I made a few changes. The problem is that the folding code is in the editor layer which does not have the concept of extensions (it is also used in the Monaco editor). So when using the extension id in the foldingStrategy setting, we have to talk about an abstract id instead of the extension id. But then users will not understand it.
-> I came up with a separate settings editor.defaultFoldingRangeProvider which is defined in the workbench layer

@daiyam

daiyam commented Oct 23, 2022

Copy link
Copy Markdown
Contributor Author

@aeschli Thank you for the update.
I don't quite understand the separation issue between the editor layer and extensions. Since src/vs/editor/contrib/folding/browser/folding.ts was only importing ExtensionIdentifier which is already imported by src/vs/editor/common/languages.ts.
But I'm ok with the changes you made. It solves the current issue.

@aeschli
aeschli merged commit 6e76e0e into microsoft:main Oct 23, 2022
@aeschli

aeschli commented Oct 23, 2022

Copy link
Copy Markdown
Contributor

It's not about the import, which is fine to use, but the description of editor.foldingStrategy: we can't write that the valid settings are indent | auto | extension-id. That sentence would not make sense in Monaco. So there's now a separate setting (editor.defaultFoldingRangeProvider), only available in VS Code, used when auto is the folding strategy.

@daiyam Thanks for your help!

@daiyam

daiyam commented Oct 23, 2022

Copy link
Copy Markdown
Contributor Author

@aeschli Thx

OK, I understand. Since it is a Monaco's property, it should conform to its definition (defined by Monaco).
All the while, I was trying to avoid an additional property...

@daiyam
daiyam deleted the custom-folding-strategy branch October 23, 2022 13:57
@depascalis

Copy link
Copy Markdown

Thanks for this. It would be even more awesome if one could restrict it to a file name or a partial file name, like *.config.xml. Or is that going to be available in your extension @daiyam?

Use cases are log files which all have their own standards, various config files which also differs a lot. The only way to distinguish them is by their file name.

@aeschli

aeschli commented Nov 11, 2022

Copy link
Copy Markdown
Contributor

Its already possible to scope the setting to a specific language

        "[javascript]": {
                "editor.defaultFoldingRangeProvider": "aeschli.ext-foldingrangeprovider"
        }

And if your files represent a certain type of xml files, you could think of defining a language for it. Thats of course something a extension would have to implement and is more than just the language definition.

@depascalis

Copy link
Copy Markdown

Alright thanks!

For future reference, here's the issue tracking scoping by filename in the Explicit Folding extension.

zokugun/vscode-explicit-folding#87

@github-actions github-actions Bot locked and limited conversation to collaborators Dec 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

editor-folding Editor code folding issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants