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

CMakePresets.json no longer supports comments #2169

Closed
alex1234567890123456789 opened this issue Oct 12, 2021 · 9 comments
Closed

CMakePresets.json no longer supports comments #2169

alex1234567890123456789 opened this issue Oct 12, 2021 · 9 comments
Labels
enhancement an enhancement to the product that is either not present or an improvement to an existing feature Feature: presets
Milestone

Comments

@alex1234567890123456789

Brief Issue Summary

In 1.7.3, you could use comments ( // ) in CMakePresets.json and everything worked fine. Starting with 1.8.0, cmake tools fails to parse CMakePresets.json if it has comments in it. While comments aren't officially allowed in json, many tools allow it, and it's not hard to support. It is also REALLY helpful to be able to document your presets file!

Debug Log

[presetController] Failed to parse CMakePresets.json: 
	SyntaxError: Unexpected token / in JSON at position 552
	at JSON.parse (<anonymous>)
	at PresetsController.parsePresetsFile (/home/nvidia/.vscode-server/extensions/ms-vscode.cmake-tools-1.8.1/dist/main.js:57684:32)
	at PresetsController.resetPresetsFile (/home/nvidia/.vscode-server/extensions/ms-vscode.cmake-tools-1.8.1/dist/main.js:57070:38)
	at async PresetsController.reapplyPresets (/home/nvidia/.vscode-server/extensions/ms-vscode.cmake-tools-1.8.1/dist/main.js:57085:9)
	at async watchPresetsChange (/home/nvidia/.vscode-server/extensions/ms-vscode.cmake-tools-1.8.1/dist/main.js:57015:13)
	at async Function.init (/home/nvidia/.vscode-server/extensions/ms-vscode.cmake-tools-1.8.1/dist/main.js:57043:9)
	at async Function.init (/home/nvidia/.vscode-server/extensions/ms-vscode.cmake-tools-1.8.1/dist/main.js:53142:35)
	at async CMakeToolsFolderController._addFolder (/home/nvidia/.vscode-server/extensions/ms-vscode.cmake-tools-1.8.1/dist/main.js:53333:22)
	at async CMakeToolsFolderController.loadAllCurrent (/home/nvidia/.vscode-server/extensions/ms-vscode.cmake-tools-1.8.1/dist/main.js:53285:17)
	at async ExtensionManager._init (/home/nvidia/.vscode-server/extensions/ms-vscode.cmake-tools-1.8.1/dist/main.js:51781:13)
	at async Function.create (/home/nvidia/.vscode-server/extensions/ms-vscode.cmake-tools-1.8.1/dist/main.js:51832:9)
	at async setup (/home/nvidia/.vscode-server/extensions/ms-vscode.cmake-tools-1.8.1/dist/main.js:52860:32)
	at async Promise.all (index 0)
	at async Promise.all (index 26)
	at async Promise.all (index 1)
@sramsesiv
Copy link

This would be really helpful! Right now our projects have the CMake Tools plugin locked back @1.7.3 until this feature is re-enabled.

@bobbrow
Copy link
Member

bobbrow commented Oct 12, 2021

We actually supported it before, but according to CMake release notes, it is not supposed to be supported: https://cmake.org/cmake/help/latest/release/3.19.html#id6

A bug was opened against us to remove the support and we did it because it matched the documented behavior: #2035

It's a bug in CMake that comments are currently allowed. We were trying to be helpful, but it turns out CMake accepts comments that are not at the top of the JSON file because there's a bug in the library they use. 🤦‍♂️

EDIT: duplicate: #2104

@bobbrow bobbrow added Feature: presets upstream Bugs related to issues in an upstream project duplicate a duplicate of an already present issue labels Oct 12, 2021
@alex1234567890123456789
Copy link
Author

Thanks @bobbrow - can it be added back with a setting to enable/disable it? It's REALLY really helpful...

@bobbrow
Copy link
Member

bobbrow commented Oct 12, 2021

I'm actually happy to go either way with this. If folks want to advocate to Kitware that JSONC should be accepted, it would probably help if folks could comment on the issue I linked above.

We can also consider a setting to allow the comments in the meantime.

@Zingam
Copy link
Contributor

Zingam commented Oct 14, 2021

We actually supported it before, but according to CMake release notes, it is not supposed to be supported: https://cmake.org/cmake/help/latest/release/3.19.html#id6

A bug was opened against us to remove the support and we did it because it matched the documented behavior: #2035

It's a bug in CMake that comments are currently allowed. We were trying to be helpful, but it turns out CMake accepts comments that are not at the top of the JSON file because there's a bug in the library they use. 🤦‍♂️

EDIT: duplicate: #2104

I think comments in json used for config files is very helpful. So CMake should support comments.

@bobbrow
Copy link
Member

bobbrow commented Oct 14, 2021

I opened a PR to add a setting for this. We can try to push it into 1.9 (which we intended to release today) if there's time or I can build a separate VSIX shortly after.

@bobbrow bobbrow modified the milestones: Tracking, 1.9.0 Oct 14, 2021
@bobbrow bobbrow added fixed (release pending) enhancement an enhancement to the product that is either not present or an improvement to an existing feature and removed upstream Bugs related to issues in an upstream project duplicate a duplicate of an already present issue labels Oct 14, 2021
@bobbrow
Copy link
Member

bobbrow commented Oct 14, 2021

The setting will be included with 1.9 hopefully later today.

@Zingam
Copy link
Contributor

Zingam commented Oct 15, 2021

Thank you. Could you please advocate for including comments in CMake and Visual Studio 22 because cross-IDE support is also the point of this feature.
I would have loved also trailing commas too but I guess config files was not what JSON was invented for. And CMake people seem to not like YML.

@andreeis
Copy link
Contributor

This fix is included in the 1.9.0 CMake Tools release. Please upgrade your extension in VSCode and let us know if you encounter any other issues.

@github-actions github-actions bot locked and limited conversation to collaborators Jan 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement an enhancement to the product that is either not present or an improvement to an existing feature Feature: presets
Projects
None yet
Development

No branches or pull requests

5 participants