This is an archive of the discontinued LLVM Phabricator instance.

[clang-format] Correctly count a tab's width in a comment
ClosedPublic

Authored by HazardyKnusperkeks on Nov 20 2022, 11:25 PM.

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptNov 20 2022, 11:25 PM
HazardyKnusperkeks requested review of this revision.Nov 20 2022, 11:25 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 20 2022, 11:25 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
rymiel accepted this revision.Nov 21 2022, 6:01 AM

The behaviour of tabs still doesn't match that of spaces, but at least this makes it idempotent.

This also fixes https://github.com/llvm/llvm-project/issues/51808, I'm closing it as duplicate in favor of the issue fixed by this patch.

This revision is now accepted and ready to land.Nov 21 2022, 6:01 AM

The behaviour of tabs still doesn't match that of spaces, but at least this makes it idempotent.

I did initially looked into that, but then asked do we want to treat tabs equally to spaces?
It would add a lot of logic, and the question how to map SpacesInLineCommentPrefix to tabs?

This also fixes https://github.com/llvm/llvm-project/issues/51808, I'm closing it as duplicate in favor of the issue fixed by this patch.

Thanks.

owenpan added inline comments.Nov 21 2022, 2:17 PM
clang/unittests/Format/FormatTestComments.cpp
659
error: reference to type 'const clang::format::FormatStyle' could not bind to an lvalue of type 'const char[61]'
      "//\t\t\t\tofMap(message.velocity, 0, 127, 0, ofGetWidth() * 0.2)");
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
HazardyKnusperkeks marked an inline comment as done.
HazardyKnusperkeks added inline comments.
clang/unittests/Format/FormatTestComments.cpp
659

Just needs the verifyFormat from D138263.

owenpan added inline comments.Nov 22 2022, 12:39 AM
clang/unittests/Format/FormatTestComments.cpp
659

I see. Do you want to stack this patch onto D138263 (which I might not be able to get to for a while) or copy the needed code from there to here?

owenpan added inline comments.Nov 22 2022, 12:41 AM
clang/unittests/Format/FormatTestComments.cpp
659

Nvm. It's stacked now.

HazardyKnusperkeks marked an inline comment as done.
HazardyKnusperkeks added a reviewer: klimek.

Pulled the verifyFormat into this change.

owenpan accepted this revision.Dec 1 2022, 1:09 AM

Thanks for unstacking it!

clang/unittests/Format/FormatTestComments.cpp
19–20

Similar to what's done in FormatTest.cpp.

HazardyKnusperkeks marked 3 inline comments as done.
owenpan accepted this revision.Dec 6 2022, 1:48 PM
This revision was landed with ongoing or failed builds.Jul 12 2023, 3:23 AM
This revision was automatically updated to reflect the committed changes.