Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: API for managing folding area #9786

Closed
rebornix opened this issue Jul 26, 2016 · 26 comments

Comments

Projects
None yet
6 participants
@rebornix
Copy link
Member

commented Jul 26, 2016

We are adding commands for fold/unfolding code in Vim and it turns out that Code's APIs are somewhat limited. All Vim's folding commands can be categorized as below.

Close/Open Folds (Done)

Vim allows users to open/close folds under cursor. Users can also decide how deep the folds are opened or closed.

These commands are partially supported, we have editor.fold, editor.unfold, editor.foldRecursively, etc and it's easy to map them to Vim commands. Besides, Code supports us to fold from level 1 to level 5 but level 6 and above is not supported and we don't have similar commands for unfold.

Vim even supports us to toggle Fold. We don't have this command in Code and we can't use editor.fold or editor.unfold to workaround as we have no idea whether we are inside a folding area.

executeCommand('editor.fold, {levels: 3});
executeCommand('editor.unfold, {levels: 3});

Fold creation/deletion

Vim allows users to create or delete folds manually. It's not possible here as Code is doing this for us.

Navigate through Folds

We can navigate to previous or next fold area through commands in Vim. If we want to implement this in Code, we need to know the info about all folds.

Fold options

We can configure max/min screen lines for a fold, max level for folds, etc in Vim. Again, we didn't expose them in Code.


There are about 20+ unique commands and 10+ options about folding and we can only implement 5 of them partially right now.

@aeschli

This comment has been minimized.

Copy link
Contributor

commented Jul 27, 2016

Discussed with Sandeep. The goal is provide commands that let you implement the VIM requirements. Unless really needed, we try not to introduce the concept of the folding regions in the extension host API.

@rebornix

This comment has been minimized.

Copy link
Member Author

commented Jul 27, 2016

@aeschli I'm Okay with hiding folding region info. If we can provide generic API to navigate/manage folding regions, then it would be perfect. If it turns out that we are adding too many Vim specific commands, maybe it can be a good time to introduce Fold.

@sandy081

This comment has been minimized.

Copy link
Member

commented Jul 27, 2016

There are already existing commands supporting some folding functionalities / operations. So, we can add more if necessary or needed. True that we are adding them as Vim needs them, but we are making sure that they are generic enough for any extension to use.

@aminroosta

This comment has been minimized.

Copy link

commented Aug 2, 2016

Unless really needed, we try not to introduce the concept of the folding regions in the extension host API.

@aeschli
I think folding info is one of the basic things extensions should be able to query, just like cursor position or number of lines in a document.

The goal is provide commands that let you implement the VIM requirements.

I don't understand what's the reason behind your decision to hide these information from extensions. I get the feel that you trying to do the extension writer jobs. If you provide the basic info for folding regions then extension writers can implement anything on top of it.

@sandy081

True that we are adding them as Vim needs them, but we are making sure that they are generic enough for any extension to use.

I think wrapping folding info with generic enough apis is not realistic right now.
Because you don't know all the use cases future extensions will need.

Having information on folding areas gives extension writers more power on how to control the editor. we can easily implement cursorMove commands you implemented, toggleFold, moveToFoldEnd, moveToFoldBegin, and anything we can possibly find in vim documentation later.

@rebornix

This comment has been minimized.

Copy link
Member Author

commented Aug 3, 2016

Vim allows us to create Fold area manually, by indentation, expression or syntax while Code is doing foding by indentation. But customizing the folding creation is not frequently used by Vim users so we should go with indentation first.

The first part is not folding command directly but they rely on folding info

  • Moving cursor across folding area. Tracked by #9619

The second part is openning/closing folds and they only require fold information.

  • za When on a closed fold: open it. When on an open fold: close it and set 'foldenable'.
  • zA When on a closed fold: open it recursively. When on an open fold: close it recursively and set 'foldenable'.
  • [z Move to the start of the current open fold.
  • ]z Move to the end of the current open fold.
  • zj Move downwards to the start of the next fold.
  • zk Move upwards to the end of the previous fold.

The third part depends on option foldlevel. 'foldlevel' is a number option: The higher the more folded regions are open.

  • zm Fold more: Subtract one from 'foldlevel'.
  • zr Reduce folding: Add one to 'foldlevel'.
  • zo Open one fold under the cursor.When a count is given, that many folds deep will be opened.
  • zc Close one fold under the cursor. When a count is given, that many folds deep are closed.
  • zv View cursor line: Open just enough folds to make the line in which the cursor is located not folded.
  • zx Update folds: Undo manually opened and closed folds: re-apply 'foldlevel', then do "zv": View cursor line.

Others are folding area manual creation/deletion and Fold options but I don't think we need to take them into consideration before we handle all commands above.

@sandy081

This comment has been minimized.

Copy link
Member

commented Aug 3, 2016

@aminroosta

Exposing folding regions (or any view regions) to the extensions is not helpful because they are based on internals. It will make difficult for extension developers to implement on top of them correctly. Instead, developing and exposing the commands will make it available to everybody and easier to use. I agree that it is not possible to develop commands / api that can meet everyone's requirements. Since it is an open community, it is always welcome to discuss/request/contribute them.

Again, just exposing folding regions will not be enough to implement commands likecursorMove, toggleFold etc... It needs enough information about editor internals.

@aminroosta

This comment has been minimized.

Copy link

commented Aug 3, 2016

@sandy081

It will make difficult for extension developers to implement on top of them correctly. Instead, developing and exposing the commands will make it available to everybody and easier to use.

With great power comes great responsibility!
Your argument is perfectly valid, it just reminds me of the same arguments Java developers use against C++ ones for accessing memory.

Exposing folding regions to the extensions is not helpful because they are based on internals.

I don't think querying the IndentRange[] array returned from computeRanges exposes editor internals.

export class IndentRange {
    _indentRangeBrand: void;
    startLineNumber:number;
    endLineNumber:number;
    indent:number;
       ....
}

Better yet just replace IndentRange[] with vscode.Range[].
And allow us to add or remove vscode.Range to the list of folding regions.

developing and exposing the commands will make it available to everybody and easier to use.

If you take the route of implementing apis with vim extensions in mind then you are doing the extension writers job, we will have future issues opening on vscode instead of the relative vim plugin repository.

That being said the commands @rebornix listed is all that i've ever used in vim and i think that's true for many many vim users and those who have used more than that will never switch from the original vim 😄

@sandy081

This comment has been minimized.

Copy link
Member

commented Aug 3, 2016

@aminroosta Agreed that exposing just folding ranges is nothing to do with the internals. But, to implement the folding commands (navigation or toggling), this data is not going to be enough and you need internal details.

I think exposing commands is the right approach for the above listed commands.

@sandy081

This comment has been minimized.

Copy link
Member

commented Aug 3, 2016

After discussing with @rebornix, Creating / Deleting foldings will not be supported because

  • Creating / Deleting folders is itself is a big editor feature with much bigger scope than Vim / editor commands.
  • Low priority from Vim extension (not mandatory)

Unless it is really required for Vim, we will de-scope it.

@aminroosta

This comment has been minimized.

Copy link

commented Aug 3, 2016

@sandy081 @rebornix
I think for navigation this data is actually enough!

All off the commands rebornix listed can be implemented.
Toggling is not an issue because if we know current line is folded or not we can implement it.

I can't think of any command from first and second part of commands that rebornix listed that can't be implemented with just this info.

If you allow adding and removing to and from fold list then the third part can be implemented as well.

@kieferrm kieferrm referenced this issue Aug 4, 2016

Closed

August Iteration Plan #10145

146 of 166 tasks complete

sandy081 added a commit that referenced this issue Aug 22, 2016

@sandy081

This comment has been minimized.

Copy link
Member

commented Aug 22, 2016

@rebornix I enhanced editor.fold and editor.unfold commands to take a number levels argument. This will help you in implementing zc and zo commands in Vim.

@rebornix

This comment has been minimized.

Copy link
Member Author

commented Aug 22, 2016

Sure I'll give it a try real quick.

@rebornix

This comment has been minimized.

Copy link
Member Author

commented Aug 22, 2016

@sandy081 I've tried your new args but it doesn't work. But I don't think it's your code's problem as editor.foldLevel1, editor.foldLevel2, etc don't work on my machine as well. I'll try and find the root cause behind it.

@rebornix

This comment has been minimized.

Copy link
Member Author

commented Aug 22, 2016

I noticed folding doesn't work in TypeScript but works in JavaScript. But I'm not seeing it take effect. For example,

function foo() {
  if (true) {
    console.log("bar");
  }
}

if we run 2zc in Vim at the third line, the code will be like

function foo() {
}

However, if I run vscode.commands.executeCommand("editor.fold", {levels: 2});, the output is still

function foo() {
  if (true) {
  }
}

It's the same as vscode.commands.executeCommand("editor.fold");

@sandy081

This comment has been minimized.

Copy link
Member

commented Aug 22, 2016

editor.fold command folds all foldings under the current cursor position till the number of levels given. This is as per the description of the command zc in Vim Roadmap

So in the above example, this folds only one level at line 3 because there are no sub foldings under it. vscode.commands.executeCommand("editor.fold", {levels: 2}); on line 1 will fold line 1 and line 2.

Let me know if I there is some mis-understanding of the documentation.

@rebornix

This comment has been minimized.

Copy link
Member Author

commented Aug 22, 2016

@sandy081 ha that makes sense.

@sandy081

This comment has been minimized.

Copy link
Member

commented Aug 22, 2016

@rebornix So the current behavior of editor.fold command is expected?

@rebornix

This comment has been minimized.

Copy link
Member Author

commented Aug 23, 2016

@sandy081 I tested it again but found that vscode.commands.executeCommand("editor.fold", {levels: 2}); only folds line 1 but not line 2.

My repro steps are : run vscode.commands.executeCommand("editor.fold", {levels: 2}); on line 1, open fold on line 1 and I found line 2 is not folded.

@sandy081

This comment has been minimized.

Copy link
Member

commented Aug 23, 2016

@rebornix See the attached video, it works as expected for me

aug-23-2016 10-04-38

First, I folded 2 levels by 2zc command and later unfolded each by zo command.

@rebornix

This comment has been minimized.

Copy link
Member Author

commented Aug 23, 2016

@sandy081 thanks for the video, it addresses the behavior well and now it's working on my machine perfectly as well!

@sandy081 sandy081 referenced this issue Aug 29, 2016

Closed

Test Scrolling and Folding API commands #11127

1 of 1 task complete
@aminroosta

This comment has been minimized.

Copy link

commented Aug 30, 2016

@sandy081 Now that you are working on folding APIs please also take a look at folding end
patterns. It should be easy to fix, I have a PR which explains what i mean by folding end patterns.

This is really deal breaker to me, every other text editor (except sublime) does it right.
VIM, Visual Studio, Intellij, Atom, Eclipse, NetBeans, XCode, Notepad++ ...

Don't make us wait another month ;-)

@sandy081

This comment has been minimized.

Copy link
Member

commented Aug 31, 2016

@aminroosta We are already in end game, so I am afraid we can push any major changes. But, I will take a look and see if they are small enough to make it.

Thanks

@cloudRoutine

This comment has been minimized.

Copy link
Contributor

commented Nov 2, 2016

Structural/Scope based code folding was a feature I implemented for VisualFSharpPowertools and one I'd love to bring over to Ionide-FSharp

The current indentation and brace based approach does a pretty good job, but it's unable to accurately capture the scopes of some of F#'s constructs and doesn't always use the same bounds for the folded region as the lexicographic scope.

The other advantage a folding API would provide us is the ability to filter which constructs are folding enabled

@aminroosta

This comment has been minimized.

Copy link

commented Jan 1, 2017

@sandy081 @aeschli
Do you guys have any plans to address the fold API issues?
Please provide your comments, so we can help by sending PRs.

@sandy081

This comment has been minimized.

Copy link
Member

commented Jan 3, 2017

@aminroosta There were not any plans for now to implement this. This need a strong use case and discussion from language protocol perspective since foldings can be language specific too.

@rebornix

This comment has been minimized.

Copy link
Member Author

commented Nov 20, 2017

This feature request will not be considered in the next 6-12 months roadmap and as such will be closed to keep the number of issues we have to maintain actionable. Thanks for understanding and happy coding!

@rebornix rebornix closed this Nov 20, 2017

@rebornix rebornix added this to the Backlog milestone Nov 20, 2017

@vscodebot vscodebot bot locked and limited conversation to collaborators Jan 5, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.