This is an archive of the discontinued LLVM Phabricator instance.

Clang-Formatting
AbandonedPublic

Authored by ps-19 on Mar 31 2022, 11:17 PM.

Diff Detail

Event Timeline

ps-19 created this revision.Mar 31 2022, 11:17 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 31 2022, 11:17 PM
ps-19 requested review of this revision.Mar 31 2022, 11:17 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 31 2022, 11:17 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
This comment was removed by ps-19.
aaron.ballman requested changes to this revision.Apr 1 2022, 4:32 AM

I'm confused -- the issue you linked is for a bug with clang-format but the changes in your patch don't modify clang-format, just modifies a test file to be formatted. I don't see how the two relate, but this doesn't seem to be addressing the issue you linked.

This revision now requires changes to proceed.Apr 1 2022, 4:32 AM
ps-19 added a comment.Apr 1 2022, 4:50 AM

okay now i understand the issue completely.
If i just mentioned that i am formatting a test file it would be correct then according to my patch.

ps-19 edited the summary of this revision. (Show Details)Apr 1 2022, 4:50 AM

okay now i understand the issue completely.
If i just mentioned that i am formatting a test file it would be correct then according to my patch.

We don't typically reformat whole files for no reason (it adds churn to the code base which makes things like git blame harder to use). We will accept a reformat like this when there's going to be immediate follow-up work in the file where having it properly formatted will make the second code review easier to understand, but that doesn't sound like the case here.

junaire added a subscriber: junaire.Apr 1 2022, 5:00 AM

You are not supposed to format the test, because the comment is actually something we called regression tests. If you reformat it (put it in the wrong place), the test will fail.

ps-19 added a comment.Apr 1 2022, 5:50 AM

Pardon, it was my mistake i didn't see that the file i was amending was actually a test file i just search "switch" statement in my script.

ps-19 abandoned this revision.Apr 1 2022, 5:59 AM

Closed as:

  1. Formating test file causes build to fail.
  2. Formatting of whole page is not accpeted in LLVM as it created problem future with commands like git blame .