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

[Ally Tiritoglu via Phabricator via cfe-commits]

atirit added a comment.

I've found yet another bug. When `AllowShortEnumsOnASingleLine` is `true` and a multiline enum is forced (by line length, trailing comma, etc.), multiple names for the enum are placed on separate lines.

Example:

  enum { A, B, C, } ShortEnum1, ShortEnum2;

Is refactored to

  enum {
      A,
      B,
      C,
  } ShortEnum1,
      ShortEnum2;

Instead of the expected

  enum {
      A,
      B,
      C,
  } ShortEnum1, ShortEnum2;

`ColumnLimit` is not causing this.
When `AllowShortEnumsOnASingleLine` is `false`, the expected behaviour occurs. This affects a unit test I added, so I'll be fixing this as well in this diff.



================
Comment at: clang/lib/Format/TokenAnnotator.cpp:3679-3680
+        if (remainingFile[whileIndex] != '\n' &&
+            (remainingFile[whileIndex] == ' ' &&
+             remainingFile[whileIndex - 1] == ' ')) {
+          remainingLineCharCount++;
----------------
curdeius wrote:
> 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?
Ah yes, that's my bad. Must have made a typo. Fixed in the next commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93938