microsoft / vscode-cpptools

Official repository for the Microsoft C/C++ extension for VS Code.
Other
5.52k stars 1.55k forks source link

Code folding didn't work as expected in directives and block comment #407

Closed LeThiHyVong closed 4 years ago

LeThiHyVong commented 7 years ago

In my code there are many places look like this

#if SOME_CONST_EXP
;//       some code
#else
      // some code
#endif

or

if SOME_CONST_EXP
;//      some code
#endif

or

/*************************************
//
//
//
//
*************************************/

EDIT (@bobbrow): (inserting request for folding on multiple consecutive single-line comments from #1443)

// Comment
// Comment
// Comment
// Comment

The code folding only work correctly between #else and #endif. From #if to next directive (#else in 1st case or #endif in 2nd case), if all line is started with ';//', the code folding didn't work. From #endif to next directive, it automatically group the code outside the directive for all time. The coding folder didn't work for 3rd, too.

codefolding

System spec:

Please correct this.

sean-mcmanus commented 7 years ago

Yes, the code folding is buggy, but it's not a bug with the C/C++ extension. We don't implement any code folding. Can you move this to the vscode team?

sean-mcmanus commented 7 years ago

Actually, it sounds like the folding behavior you are seeing is "by design". The fixing of this is being tracked at https://github.com/Microsoft/vscode/issues/3422 . VS Code implements the folding behavior and there isn't an API yet to allow language-aware folding.

LeThiHyVong commented 6 years ago

According to https://github.com/Microsoft/vscode/issues/3422#event-1582353384, please re-open this issue. Thanks.

hoffmael commented 6 years ago

This would be a great feature and bug fix to have and essential for the code base I work on which relies on many preprocessor switches to support multiple builds. Diming the inactive code with "C_Cpp.dimInactiveRegions": true is a good first step, but having the ability to collapse and hide the inactive code region altogether is even better. Eclipse even has the option to collapse inactive regions by default when opening a file by default.

Here's a screenshot from 0.17.4 showing the various issues with folding currently. image

sean-mcmanus commented 6 years ago

+1 I hit this issue sometimes with our cross platform code.

intijk commented 6 years ago

C++ folding without supporting this feature basically is useless for cross platform project.

Flamefire commented 6 years ago

@hoffmael Great example why this is required. I ran into pretty much the same thing. What is even worse: It breaks folding of whole namespaces making that completely useless.

Note why this breaks (https://github.com/Microsoft/vscode/issues/57696#issuecomment-418108134):

The folding strategy that you are seeing is based purely on the indentation of lines. A folding region starts when a line has a smaller indent than one or more following lines and ends when there is a line with the same or smaller indent.

akbyrd commented 6 years ago

+1 We definitely need language based folding. The current indentation based implementation is completely unnatural with preprocessor directives and public/private specifiers where a common pattern is to leave them left aligned with no indentation. #endif lines end up as folding nodes and weird, ugly things happen.

A common part of my workflow is to Fold All to get an overview of a file. With indentation based folding this goes to hell.

Here's some example code (note the placement of the folding buttons) image

Here's how it actually folds. Not very useful. image

Here's how I'd expect it to fold. (Though, honestly I wish the braces and ... got collapsed onto the same line as the function.) image

britalmeida commented 6 years ago

Here is an example of how the braces look collapsed on the same line (although they weren't originally):

inline_folding

This is quite important to get the code to fit the screen in the case of big files with many functions.

akbyrd commented 5 years ago

This is a big enough daily nuisance that I started looking at doing it myself, but it looks like most of the language server functionality is bundled up into binaries that get downloaded: Microsoft.VSCode.CPP.Extension.exe & Microsoft.VSCode.CPP.IntelliSense.Msvc.exe.

Any guidance on whether it's possible to get any sort of scope or AST information out of the language server in spite of this?

sean-mcmanus commented 5 years ago

@akbyrd The code for the Microsoft.VSCode.CPP processes is closed source and there's no way to get any AST info (i.e. so you'd need to be a Microsoft employee or contractor). I think we just need to respond to the textDocuent/foldingRange request and run our lexer to compute the ranges. If you can't wait for us, you might be able to write a separate extension that uses an open source lexer to do this.

akbyrd commented 5 years ago

I figured as much. I'll roll with a custom extension for the time being. Do you have any information regarding an ETA for folding support?

sean-mcmanus commented 5 years ago

@akbyrd We don't have any ETA yet, but as soon as we do we'll add this issue to a Milestone with a target month. I would prefer to get this implemented by March, but other issues, such as improving our configuration experience, could get higher priority, and we haven't finished Find All References yet (our top requested feature).

akbyrd commented 5 years ago

@sean-mcmanus With March and April sliding by is there any possibility of nudging this in for the next milestone? Folding is a daily usability issue and language aware folding would be a massive improvement for many people even though they aren't actively complaining about it.

Flamefire commented 5 years ago

+1 for that. What would also be great (as highly related) is some view in which class/namespace/scope you are with the current cursor. This information is probably known, when code folding is implemented and helps for large files...

sean-mcmanus commented 5 years ago

@akbyrd I brought up this issue in our planning meeting, but we still don't have a milestone. The next milestone is probably booked with Find All References, semantic colorization, UI/config work, and performance fixes.

@Flamefire Your issue appears to be tracked by https://github.com/Microsoft/vscode-cpptools/issues/2230 (plus breadcrumbs enabled).

akbyrd commented 5 years ago

If anyone else wants a short term solution I slammed together a folding plugin: https://github.com/akbyrd/vsc-cpp-folding

Shadowblitz16 commented 5 years ago

+1 I have OCD and cannot stand to program with visual studio code right now. code folding should act the same as it does in visual studio 2019 void function() { ... }

janwilmans commented 4 years ago

image

I don't care who fixes it, could you please resolve it amongst yourselves, fix the relevant issue and close the other. see https://github.com/microsoft/vscode/issues/84977

aeschli commented 4 years ago

@janwilmans the comment

Yes, the code folding is buggy, but it's not a bug with the C/C++ extension. We don't implement any code folding. Can you move this to the vscode team?

was made when there was no API in VSCode for extensions to provide folding regions.

In the meantime there is and as you can see from the followup discussion, the C++ team is aware of the request but has not yet found time to work on it.

KaeLL commented 4 years ago

LMAO 2 weeks away from 3 years since this issue was opened, not counting the fact that you can't have a C/C++ editor without this for any coding other than hello-world level code and still nothing. Amazing. I'm not blaming the dev's of the cpp extension or anything, just wanted to put it into perspective.

akbyrd commented 4 years ago

I'm not blaming the dev's of the cpp extension

vscode doesn't work with Windows features (Aero Snap). vscode-cpp doesn't work with vscode features (code folding). Something in the process is fundamentally broken when a company can't keep its own products working together smoothly.

KaeLL commented 4 years ago

I'm not blaming the dev's of the cpp extension

vscode doesn't work with Windows features (Aero Snap). vscode-cpp doesn't work with vscode features (code folding). Something in the process is fundamentally broken when a company can't keep its own products working together smoothly.

The aero snap part you can thank Electron for that. It's also the reason why there's no "its own products working together" thing.

sean-mcmanus commented 4 years ago

@KaeLL Yeah, this is currently the 8th top voted feature so hopefully we'll get it implemented some time next year. Last year we implemented Find All Refs and Rename which were the top issues, as well as localization, semantic colorization, doc comments, config UI, hierarchical outline/breadcrumbs, etc., with current work being done on improved multi-root support, a Makefile config provider (with improvements to the CMake Tools extension), improvements to our test coverage, and other bug fixes (i.e. https://github.com/microsoft/vscode-cpptools/milestone/44 ) in preparation for version "1.0".

intijk commented 4 years ago

@KaeLL Yeah, this is currently the 8th top voted feature ...

Hi @sean-mcmanus , could you let me know where to see this rank?

sean-mcmanus commented 4 years ago

@intijk https://github.com/Microsoft/vscode-cpptools/issues?q=is%3Aissue+is%3Aopen+sort%3Areactions-%2B1-desc . I excluded the "debugger" features in the counting.

yuchongbo commented 4 years ago

this issue is opened since 2016 and mentioned by so many people again and again, I am curious why Microsoft Vscode developers have been thinking it not an issue which should be fixed. what's the purpose MS gets the VScode out? can't this be easy for use?

intijk commented 4 years ago

@JYU2018

I kind of think this might be some challenge on the compiler tech that Microsoft people do not have the ability to do it. Anyway if there is a challenge, some idea share the hard point would also welcome.

The real sad thing I've notice about vscode project is, every time new release out, they advertise about how many tickets on the github is resolved, but from my experience, they just diligently close issues instead of really solve them.

Flamefire commented 4 years ago

I actually don't think this is very hard to implement. The ColorRangeProvider (or what it's name was) already has the ranges (or at least some of them) to e.g. shade inactive regions. One could picky-back on that for a quick-fix.

Or with a different approach: Simply detect the preprocessor macros #if, #else, #elif, #endif and return that as ranges. Won't be to performant though depending on how often the ranges are requested.

If someone got some time to do it and open a PR, than maybe it will be accepted. I currently haven't unfortunately. Otherwise there are already plugins out there based on regex to provide ranges. Check them out! :)

sean-mcmanus commented 4 years ago

This just hasn't gotten on our schedule to start working on. Different people have different opinions on what is a priority or not.

yuchongbo commented 4 years ago

Anyway, I do believe this is the BASIC feature which VScode should have at first time the VScode release.

Flamefire commented 4 years ago

Anyway, I do believe this is the BASIC feature which VScode should have at first time the VScode release.

If this was the case: Why are so few people interested in this? The metric for "interested" is the number of :+1: on this issue. There are 33 here and 191 in the top issue, see the comment https://github.com/microsoft/vscode-cpptools/issues/407#issuecomment-560983100

So either convince ~200 people to vote for this, implement it yourself (this is a foss project afterall), use an other extension for code folding (there are some) or wait till this trickles to the top of the priority queue.

ghost commented 4 years ago

I think Atom managed to support this type of folding with the tree-sitter parser, if that's helpful to anyone working on this.

https://blog.atom.io/2018/03/15/atom-1-25.html#improved-syntax-highlighting-and-code-folding

sean-mcmanus commented 4 years ago

@gbhojraj Yeah, there's a thread/issue on VS Code that discusses potentially moving to tree sitter, so then we'd get this fixed "for free". The relevant VS Code issues are https://github.com/microsoft/vscode/issues/77140 and https://github.com/microsoft/vscode/issues/50140 .

jpramosi commented 4 years ago

If someone is looking for a solution, i have created this extension https://github.com/reapler/vscode-cfold because the preprocessor directives has driven me mad :)

HunterZ commented 4 years ago

I want to make sure that this bug covers the ability to automatically (on either file open, or user command) fold inactive code regions (especially due to preprocessor macros). The bug title+description don't make this clear, so I wrote bug #5020 which was closed as a duplicate of this. Thanks.

soul4soul commented 4 years ago

After a year of being annoyed I finally came to file this bug. I wasn't too surprised to see many beat me to it. I miss C/C++ code folding which I have come to love from VS. I'm excited to see this has been moved to in progress. 🐱‍🏍

KaeLL commented 4 years ago

Over 3 years later.... This feature alone is the only reason I use Eclipse for a component in my project that makes extensive usage of conditional compilation macros.

sean-mcmanus commented 4 years ago

Accurate C++ code folding is available in https://github.com/microsoft/vscode-cpptools/releases/tag/0.28.0-insiders2 . Let us know if anyone find any issues with it.

akbyrd commented 4 years ago

Issue 1 Code folding does not work on new files set to C++ language mode. They revert to indentation based folding.

image

Issue 2 Switch cases with braces fold differently than everything else. Everything else folds up to the keyword. Cases only fold up to the opening brace. Note the position of the folding buttons in this image. The second arrows is next to switch while the third arrow is on the brace after case.

image

And here is the inconsistency that results

image image

Issue 3 Switch cases without braces do not fold.

image

There's room for debate here, as the case contents are not a scope, but personally I'd like to be able to fold them as they are still a logical block of code, if not a lexical scope.

Issue 4 It's obnoxious code, but this if doesn't fold to a closing brace at all.

image image

Issue 5 Multi-line macros do not fold.

image

I have some quick and dirty tests here in the form of .cpp files with various code examples that should or should not fold.

sean-mcmanus commented 4 years ago

@akbyrd Issue 1 has the same root cause as https://github.com/microsoft/vscode-cpptools/issues/4143, i.e. none of our language server features will work until the file is saved.

Issue 2 is because "case:" has no '{' associated with it in the C++ language like the other if/switch blocks, so it's treated like all other {} blocks, e.g. your code could be

case 0:
int i;
{
}
break;

The {... looks odd, but we can't do what VS does until VS Code makes changes (which has existing issues tracking it).

Issue 3: I filed a feature request on VS at https://developercommunity.visualstudio.com/idea/1020490/c-intellisense-add-code-folding-for-case-regions.html . Our implementation is shared with VS.

Issue 4: What is the result you expect in this case? It appears to fold correctly to me.

Issue 5: I've filed a feature request on VS at https://developercommunity.visualstudio.com/idea/1020893/c-intellisense-multiline-macros-dont-generate-a-co.html . I don't see it in your screenshot, but the "if" block is supposed to generate a code folding region (so let us know if you're not getting that).

akbyrd commented 4 years ago

Issue 1 has the same root cause as #4143, i.e. none of our language server features will work until the file is saved.

I copy code into a temp file quite frequently. This was the very first issue I encountered.

Issue 2 is because "case:" has no '{' associated with it in the C++ language like the other if/switch blocks,

Indeed, case doesn't introduce a scope in C++, but in your AST you likely have a case node followed by a scope node. That's a pattern you can handle to associate the braces with the case when they appear together. This is essentially how I've handled it (but at the token level) and it has worked quite well.

Issue 4: What is the result you expect in this case? It appears to fold correctly to me.

Apologies. You are correct. However, it is incorrect when the the active preprocessor block is reversed.

image image

In this case I expect it to fold to the closing brace on line 39, which is in the active preprocessor region. If it were doing so, lines 38 and 39 would not be visible.

Perhaps it's assuming the first preprocessor block is the active one?

Issue 5: I don't see it in your screenshot, but the "if" block is supposed to generate a code folding region (so let us know if you're not getting that).

The if in that example does not generate a code folding region.

sean-mcmanus commented 4 years ago

@akbyrd Oh, issue 4 is unexpected because it doesn't repro with VS's implementation, so it implies that we somehow regressed that case or did something differently with our changes. I've filed bug https://github.com/microsoft/vscode-cpptools/issues/5429 .

I wasn't reproing issue 5 because my repro was slightly different with { on the same line as the if. I've filed VS bug https://developercommunity.visualstudio.com/content/problem/1021278/c-intellisense-code-folding-works-in-a-macro-defin.html .

Did you want us to add a setting to disable Code Folding with our extension so it can be provided by the vsc-cpp-folding extension? We thought about that but were going to wait for complaints before adding another enable/disable setting.

akbyrd commented 4 years ago

As a backup, yes, that would be helpful.

That said, I'd much rather use the official plugin. It's already 90% there, so I expect I'll be able to use it. The new file behavior and macro folding are the only blockers I've encountered so far.

Also, to clarify Issue 5, I would expect that the macro contents themselves fold, independent of whether the if block in the macro folds. I'm not terribly worried about the if, personally.

In the example, I'd expect that lines 20-21 and 23-27 are foldable.

sean-mcmanus commented 4 years ago

A C_Cpp.codeFolding ("Enabled"/"Disabled") setting has been added for 0.28.0 (not released yet): https://github.com/microsoft/vscode-cpptools/pull/5435 .

There is no ETA on when the blocking issues are going to be fixed.

KaeLL commented 4 years ago

Accurate C++ code folding is available in https://github.com/microsoft/vscode-cpptools/releases/tag/0.28.0-insiders2 . Let us know if anyone find any issues with it.

Ok. I understand that this issue is actually an umbrella for a lot of small issues related to how C/C++ code should be folded. I also believe the following problem, which are different issues about the same problem, indentation folding, is the most prominent of the bunch: https://github.com/microsoft/vscode-cpptools/issues/1584 https://github.com/microsoft/vscode-cpptools/issues/2116 https://github.com/microsoft/vscode-cpptools/issues/3589 https://github.com/microsoft/vscode/issues/57696

I just checked, it's still not fixed, and indentation-based folding is still at play here, so I'm not sure what you mean by "Accurate"?

code

akbyrd commented 4 years ago

It's only in the insiders build of the plugin. If you follow the link it has instructions for enabling the insiders branch of the cpp plugin.

KaeLL commented 4 years ago

It's only in the insiders build of the plugin. If you follow the link it has instructions for enabling the insiders branch of the cpp plugin.

settings

Here's the link to my settings.json file backup I update every time I change something in case you think I just added this setting. It's been enabled for quite a while. https://pastebin.com/txMGa5Et

elahehrashedi commented 4 years ago

This issue is fixed in 0.28.0. 0.28.0 release: https://github.com/microsoft/vscode-cpptools/releases/tag/0.28.0

sean-mcmanus commented 4 years ago

@KaeLL Are you still reproing those issues with 0.28.0? I'm not able to repro those issues. Could you file a new issue with more repro details?

image