Userbla added a comment.
In D93938#2833045 <https://reviews.llvm.org/D93938#2833045>, @Userbla wrote:
> It fails in the second case because we don't respect the 'AfterEnum = true'
> and collapse the brace. It appears there is buggy logic in the
> `remainingLineCharCount` stuff which others have already been commenting on
Apologies, late to the party here... this fix suggested by curdeius does indeed
fix the issue:
> 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:3679-3680
+ if (remainingFile[whileIndex] != '\n' &&
+ (remainingFile[whileIndex] == ' ' &&
+ remainingFile[whileIndex - 1] == ' ')) {
+ remainingLineCharCount++;
----------------
atirit wrote:
> 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.
> 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?
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
