[PATCH] D93938: [clang-format] Fixed AfterEnum handling

Marek Kurdej via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Mar 17 10:01:52 PDT 2021


curdeius added inline comments.


================
Comment at: clang/lib/Format/TokenAnnotator.cpp:3679-3680
+        if (remainingFile[whileIndex] != '\n' &&
+            (remainingFile[whileIndex] == ' ' &&
+             remainingFile[whileIndex - 1] == ' ')) {
+          remainingLineCharCount++;
----------------
atirit wrote:
> curdeius wrote:
> > I don't really understand these conditions on spaces. Could you explain your intent, please?
> > You really need to add specific tests for that, playing with the value of ColumnLimit, adding spacing etc.
> Repeated spaces, e.g. `enum {   A,  B, C } SomeEnum;` are removed during formatting. Since they wouldn't be part of the formatted line, they shouldn't be counted towards the column limit. Only one space need be considered. Removed spaces, e.g. `enum{A,B,C}SomeEnum;` are handled by the fact that `clang-format` runs multiple passes. On the first pass, spaces would be added. On the second pass, assuming the line is then too long, the above code would catch it and break up the enum.
> 
> I'll add unit tests to check if spaces are being handled correctly.
Since you use `== ' '` twice, `remainingLineCharCount` will count only consecutive spaces, right?
But you want to count other characters, no?
So, IIUC, the condition you want is `rF[wI] != '\n' && !(rF[wI] == ' ' && rF[wI - 1] == ' ')` (mind the negation). It will count characters other than a newline and it will only count a series of consecutive spaces as one char. WDYT?


================
Comment at: clang/lib/Format/TokenAnnotator.cpp:3681
+             remainingFile[whileIndex - 1] == ' ')) {
+          remainingLineCharCount++;
+        }
----------------
Nit.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93938/new/

https://reviews.llvm.org/D93938



More information about the cfe-commits mailing list