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

Add /** */ to cpp/language-configurations.json #211202

Merged
merged 9 commits into from May 15, 2024

Conversation

sean-mcmanus
Copy link
Contributor

Fixes microsoft/vscode-cpptools#12249 .

This was removed in 98fa77a .

Then PR #160357 added /* */ autoClosingPair.

This fixes the issue at microsoft/vscode-cpptools#12249 .

This was removed in microsoft@98fa77a .

Then PR microsoft#160357 added /* */ autoClosingPair.
@sean-mcmanus
Copy link
Contributor Author

@bobbrow @Colengms Is there any issue with this?

@sean-mcmanus
Copy link
Contributor Author

@bobbrow I talked this over with Colen -- this causes users with setting "C_Cpp.doxygen.generatedStyle": "/**" to not get onType doxygen generation. Is that okay or what?

@bobbrow
Copy link
Member

bobbrow commented Apr 24, 2024

@bobbrow I talked this over with Colen -- this causes users with setting "C_Cpp.doxygen.generatedStyle": "/**" to not get onType doxygen generation. Is that okay or what?

If the user has that setting set, can we replace the language configuration with one that doesn't specify /** and make it work?

@sean-mcmanus
Copy link
Contributor Author

sean-mcmanus commented Apr 24, 2024

If the user has that setting set, can we replace the language configuration with one that doesn't specify /** and make it work?

@bobbrow We can? How? Even if users have the generationStyle set to /**, they'd still get the buggy indentation for comments that are not before a declaration which auto-generates doxygen, i.e. in a function.

It's possible our doxygen generation could be fixed by looking for the */ change and then looking if the characters beforehand are /** -- it currently only looks for /**.

@sean-mcmanus
Copy link
Contributor Author

sean-mcmanus commented May 2, 2024

@bobbrow I have a PR/fix for Doxygen comment generation that handles the /** */ case that results from this change.

UPDATE: The fix PR is in for 1.21.0.

@sean-mcmanus
Copy link
Contributor Author

@alexr00 What's the timeline on getting this reviewed?

@alexr00
Copy link
Member

alexr00 commented May 14, 2024

@sean-mcmanus can you walk me through what has changed since #77008 (comment) that makes this change better now?

@sean-mcmanus
Copy link
Contributor Author

@sean-mcmanus can you walk me through what has changed since #77008 (comment) that makes this change better now?

@alexr00 Yeah, VS Code added the editor.autoClosingComments setting. Also, we fixed an issue with our Doxygen generation code for our next release. So, our team isn't aware of any downside to this change.

@sean-mcmanus
Copy link
Contributor Author

sean-mcmanus commented May 14, 2024

@sean-mcmanus can you walk me through what has changed since #77008 (comment) that makes this change better now?

@alexr00 Yeah, VS Code added the editor.autoClosingComments setting. Also, we fixed an issue with our Doxygen generation code for our next release. So, our team isn't aware of any downside to this change.

Also, after that comment was made, we added the /**/ completion: see #160357 .

@alexr00 alexr00 added this to the May 2024 milestone May 15, 2024
@alexr00 alexr00 enabled auto-merge (squash) May 15, 2024 09:03
Copy link
Member

@alexr00 alexr00 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thank you!

@alexr00 alexr00 merged commit 739d480 into microsoft:main May 15, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JSDoc-style multi-line comments are wrongly indented
6 participants