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

Reply via email to