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

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

Closed
LeThiHyVong opened this issue Dec 13, 2016 · 57 comments
Closed
Assignees
Labels
Feature Request fixed Check the Milestone for the release in which the fix is or will be available. Language Service
Milestone

Comments

@LeThiHyVong
Copy link

LeThiHyVong commented Dec 13, 2016

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:

  • Windows 7 SP1 x86
  • VSCode 1.7.2
  • Plugin 0.9.3

Please correct this.

@LeThiHyVong LeThiHyVong changed the title No code folding after #if directive and incorrect folding after #endif Code folding didn't work as expected in directives and block comment Dec 13, 2016
@sean-mcmanus
Copy link
Collaborator

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
Copy link
Collaborator

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

@LeThiHyVong
Copy link
Author

According to microsoft/vscode#3422 (comment), please re-open this issue.
Thanks.

@hoffmael
Copy link

hoffmael commented Jun 1, 2018

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
Copy link
Collaborator

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

@intijk
Copy link

intijk commented Aug 30, 2018

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

@Flamefire
Copy link

@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 (microsoft/vscode#57696 (comment)):

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.

@bobbrow bobbrow added this to the On Deck milestone Sep 4, 2018
@akbyrd
Copy link

akbyrd commented Sep 10, 2018

+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
Copy link

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
Copy link

akbyrd commented Jan 4, 2019

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
Copy link
Collaborator

@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
Copy link

akbyrd commented Jan 4, 2019

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
Copy link
Collaborator

@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).

@KaeLL
Copy link

KaeLL commented Apr 2, 2020

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.

@Colengms Colengms mentioned this issue Apr 16, 2020
@Colengms Colengms added the fixed Check the Milestone for the release in which the fix is or will be available. label Apr 23, 2020
@sean-mcmanus
Copy link
Collaborator

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
Copy link

akbyrd commented May 7, 2020

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
Copy link
Collaborator

sean-mcmanus commented May 7, 2020

@akbyrd Issue 1 has the same root cause as #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
Copy link

akbyrd commented May 7, 2020

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
Copy link
Collaborator

sean-mcmanus commented May 7, 2020

@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 #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
Copy link

akbyrd commented May 7, 2020

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
Copy link
Collaborator

sean-mcmanus commented May 8, 2020

A C_Cpp.codeFolding ("Enabled"/"Disabled") setting has been added for 0.28.0 (not released yet): #5435 .

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

@KaeLL
Copy link

KaeLL commented May 13, 2020

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:
#1584
#2116
#3589
microsoft/vscode#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
Copy link

akbyrd commented May 13, 2020

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
Copy link

KaeLL commented May 13, 2020

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
Copy link
Contributor

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
Copy link
Collaborator

@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

@sean-mcmanus
Copy link
Collaborator

@ret394 @KaeLL Yeah, our extension doesn't process unsaved files, so it'll fall back to use the previous code folding behavior provided by VS. You can upvote the unsaved file issue at #3343 .

@KaeLL
Copy link

KaeLL commented May 15, 2020

The file was saved, for heaven's sake.
Open VS, open a new file, write that code, save it as trash.c, and there'll be no collapse arrow for the curly brackets (the main function block of code), only the individual preprocessor directives.
Indent the directives by 1 tab, and the collapse arrow shows up for the curly brackets and disappear for the directives, proving it's still indentation based folding. I'm using the 0.28.0 version of the extension, with the 1.45.1 version of VSCode on Clear Linux.
Here's my settings.json

{
	"editor.fontFamily": "'Cascadia Code', 'Source Code Pro Medium', 'Ubuntu Mono', 'Cascadia Mono'",
	"editor.fontSize": 16,
	"editor.accessibilitySupport": "off",
	"editor.wordWrap": "on",
	"editor.renderControlCharacters": true,
	"editor.renderWhitespace": "all",
	"editor.smoothScrolling": true,
	"editor.fontLigatures": true,
	"editor.cursorSmoothCaretAnimation": true,
	"editor.minimap.enabled": false,
	"editor.quickSuggestionsDelay": 0,
	"editor.renderLineHighlight": "all",

	"[c]": {
		"editor.wordWrapColumn": 8000,
		"editor.acceptSuggestionOnEnter": "off",
		"editor.detectIndentation": false,
		"editor.dragAndDrop": false,
		"editor.emptySelectionClipboard": false,
		"editor.insertSpaces": false,
		"editor.showFoldingControls": "always",
		"editor.wordBasedSuggestions": false,
		"editor.acceptSuggestionOnCommitCharacter": false,
		"editor.autoClosingBrackets": "beforeWhitespace",
		"editor.autoClosingQuotes": "beforeWhitespace",
		"editor.autoSurround": "brackets",
		"editor.copyWithSyntaxHighlighting": false,
		// "editor.defaultFormatter": "ms-vscode.cpptools",
		"editor.foldingStrategy": "indentation",
		// "editor.formatOnType": true,
		"editor.largeFileOptimizations": false,
	},

	"files.enableTrash": false,
	// "files.insertFinalNewline": true,
	"files.trimFinalNewlines": true,
	"files.trimTrailingWhitespace": true,
	"files.hotExit": "off",
	// "files.watcherExclude": {
	// //	"**/.**":true
	// 	"**/*.old/**":true,
	// },
	// "search.exclude": {
	// 	"**/*.old/**":true,
	// 	"**/[.]":true,
	// 	"**/esp-idf/**":true
	// },

	"workbench.editor.enablePreview": false,
	"workbench.editor.enablePreviewFromQuickOpen": false,
	"workbench.editor.highlightModifiedTabs": true,
	"workbench.settings.editor": "json",
	"workbench.settings.useSplitJSON": true,
	"workbench.settings.openDefaultSettings": true,
	"workbench.colorTheme": "Default High Contrast",
	"workbench.colorCustomizations": {
		// "editorWhitespace.foreground": "#000000a0",
		"editorWhitespace.foreground": "#ffffff50",
		// "editor.selectionBackground": "#ffffffc2",
		// "editor.selectionHighlightBackground": "#00000000"
	},

	"window.menuBarVisibility": "toggle",
	"terminal.integrated.fontFamily": "'Cascadia Mono', 'Source Code Pro Medium', 'Ubuntu Mono'",
	"terminal.integrated.cursorBlinking": true,
	"terminal.integrated.cursorStyle": "line",
	"terminal.integrated.fontSize": 16,
	"terminal.integrated.scrollback": 18446744073709551615,

	"C_Cpp.clang_format_path": "/usr/bin/clang-format",

	"C_Cpp.experimentalFeatures": "Enabled",
	"C_Cpp.updateChannel": "Insiders",
	"C_Cpp.loggingLevel": "Information",

	"diffEditor.ignoreTrimWhitespace": false,
	"breadcrumbs.enabled": true,
	"breadcrumbs.symbolSortOrder": "type",
	"files.associations": {
		"*.h": "c"
	},
	"window.zoomLevel": 0,
	"C_Cpp.autoAddFileAssociations": false,
	"C_Cpp.clang_format_sortIncludes": false,
	"C_Cpp.default.browse.limitSymbolsToIncludedHeaders": false,
	"C_Cpp.default.cppStandard": "c++20",
	"C_Cpp.default.cStandard": "c11",
	"C_Cpp.default.intelliSenseMode": "gcc-x64",
	"C_Cpp.suggestSnippets": false,
	"C_Cpp.vcpkg.enabled": false,
}

I have nothing more to add. The issue exists, but if it's happening only to me, then it's not happening and it's fixed for all intents and purposes, and I no longer care enough to pursue to the end. Might as well close the thread. Good day.

@sean-mcmanus
Copy link
Collaborator

@KaeLL It's caused by your setting "editor.foldingStrategy": "indentation". The issue should be fixed if you remove that.

@yuchongbo
Copy link

I tried using the .28 version cpp extension, vscode version 1.45.1 and found an interesting thing.
the case block cannot be folded in [switch] statement in an exist .cpp file, but when I copied the same content to a new created cpp file, case block folding works!
switch_case_folding

@akbyrd
Copy link

akbyrd commented May 16, 2020

VS Code seems to cache the folding regions of files. When you open a folder or workspace it saves your open files and some other state. I think folding is part of that state. I haven't played around with it enough to know how to reliably reset that state. Maybe editing the file? Closing all files and clearing editor history? Just a thought.

@sean-mcmanus
Copy link
Collaborator

@yuchongbo The case folding is being tracked by #5523 . The fact that it starts working when it's copied to unsaved/new file is due to the fact that our extension only works on "file"-based URIs (and not the "untitled" URI scheme), which is tracked by #3343 .

@github-actions github-actions bot locked and limited conversation to collaborators Oct 19, 2020
@sean-mcmanus
Copy link
Collaborator

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Feature Request fixed Check the Milestone for the release in which the fix is or will be available. Language Service
Projects
None yet
Development

No branches or pull requests