This is an archive of the discontinued LLVM Phabricator instance.

[clang-format] Add option to decorate reflowed block comments
Needs RevisionPublic

Authored by apwadkar on Mar 26 2023, 3:08 PM.

Details

Summary

Added the DecorateReflowComments option to control whether '* ' are added
to the beginnings of continuation lines for block comments.

Diff Detail

Event Timeline

apwadkar created this revision.Mar 26 2023, 3:08 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 26 2023, 3:08 PM
apwadkar requested review of this revision.Mar 26 2023, 3:08 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 26 2023, 3:08 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript

You need to regenerate the documentation after changing Format.h

MyDeveloperDay added a project: Restricted Project.

You need to add unit tests

MyDeveloperDay requested changes to this revision.Mar 26 2023, 3:12 PM
This revision now requires changes to proceed.Mar 26 2023, 3:12 PM

Sorry about the premature review, this is my first commit, and I didn't realize it wouldn't create a draft first.

  • For rebuilding the docs, I assume I need to add -DLLVM_BUILD_DOCS=true to CMake and then run ninja -C build?

no you need to run

clang/doc/tools/dump_format_style.py

This will regnerate ClangFormatStyleOptions.rst from the Format.h change you have here, you then need to include that rst file in the review

MyDeveloperDay added inline comments.Mar 27 2023, 1:38 AM
clang/include/clang/Format/Format.h
2022

you are not setting the default value for this so it could be uninitialized, you need to set that in the LLVMStyle section

you need to ensure the == operator has been updated

you should be adding a CHECK_PARSE_BOOL() unit test at a minimum but also some verifyFormat checks in clang/unittest/Format

clang/lib/Format/BreakableToken.cpp
406

So if set to false it will never add the decoration. But when set to true it still will not, if it's the first line.

This is not what I'd expect reading the documentation.

If you want to pursue this I think you should go for an enum with Never, Always, and <insert name here> (what we currently have).

apwadkar added inline comments.Mar 28 2023, 8:11 PM
clang/lib/Format/BreakableToken.cpp
406

So if set to false it will never add the decoration. But when set to true it still will not, if it's the first line.

This is not what I'd expect reading the documentation.

Yeah, I think that makes sense. That's what I was thinking while working on the unit tests, because I realized this was breaking the existing style.