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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits