[PATCH] D150403: [clang-format] Adjust braced list detection (try 2)

2023-07-27 Thread Galen Elias via Phabricator via cfe-commits
galenelias added a comment. In D150403#4539874 , @owenpan wrote: > This seems to cause a regression. See > https://github.com/llvm/llvm-project/issues/64134. @galenelias any idea? I will take a look. The logic I added is trying to distinguish `{ } {` b

[PATCH] D150403: [clang-format] Adjust braced list detection (try 2)

2023-07-27 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment. Herald added a subscriber: wangpc. This seems to cause a regression. See https://github.com/llvm/llvm-project/issues/64134. @galenelias any idea? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150403/new/ https://reviews.ll

[PATCH] D150403: [clang-format] Adjust braced list detection (try 2)

2023-05-22 Thread Owen Pan via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG6dcde658b238: This is a retry of https://reviews.llvm.org/D114583, which was backed (authored by galenelias, committed by owenpan). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

[PATCH] D150403: [clang-format] Adjust braced list detection (try 2)

2023-05-22 Thread Owen Pan via Phabricator via cfe-commits
owenpan accepted this revision. owenpan added a comment. In D150403#4362403 , @galenelias wrote: > Galen Elias > > I'm not sure if there is somewhere I should be putting that in the summary or > diff itself, or just here in the comments? Just here in

[PATCH] D150403: [clang-format] Adjust braced list detection (try 2)

2023-05-22 Thread Galen Elias via Phabricator via cfe-commits
galenelias updated this revision to Diff 524484. galenelias edited the summary of this revision. galenelias added a comment. Thanks for the additional review comments. Hopefully everything if fixed. In D150403#4358845 , @owenpan wrote: > Can you put you

[PATCH] D150403: [clang-format] Adjust braced list detection (try 2)

2023-05-20 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment. In D150403#4350325 , @galenelias wrote: > In D150403#4347323 , > @HazardyKnusperkeks wrote: > >> We'll wait a bit, if someone might have a comment. And (at least I) need >> name and ema

[PATCH] D150403: [clang-format] Adjust braced list detection (try 2)

2023-05-20 Thread Emilia Kond via Phabricator via cfe-commits
rymiel added a comment. This patch also fixes https://github.com/llvm/llvm-project/issues/52911 (which is probably a duplicate anyway) Comment at: clang/lib/Format/UnwrappedLineParser.cpp:583-584 + ProbablyBracedList || + NextTok->is(tok::l_brace) &&

[PATCH] D150403: [clang-format] Adjust braced list detection (try 2)

2023-05-17 Thread Galen Elias via Phabricator via cfe-commits
galenelias added a comment. In D150403#4347323 , @HazardyKnusperkeks wrote: > We'll wait a bit, if someone might have a comment. And (at least I) need name > and email for the commit. Name: Galen Elias Email: galenelias at gmail dot com CHANGES SINCE

[PATCH] D150403: [clang-format] Adjust braced list detection (try 2)

2023-05-16 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D150403#4343708 , @galenelias wrote: > Thanks @HazardyKnusperkeks! I don't have commit access, so will need someone > to land this for me. We'll wait a bit, if someone might have a comment. And (at least I) need

[PATCH] D150403: [clang-format] Adjust braced list detection (try 2)

2023-05-15 Thread Galen Elias via Phabricator via cfe-commits
galenelias added a comment. Thanks @HazardyKnusperkeks! I don't have commit access, so will need someone to land this for me. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150403/new/ https://reviews.llvm.org/D150403 ___ cfe-commits mailing

[PATCH] D150403: [clang-format] Adjust braced list detection (try 2)

2023-05-15 Thread Galen Elias via Phabricator via cfe-commits
galenelias marked 3 inline comments as done. galenelias added a comment. Thanks for the review and feedback. Sorry about the unfortunate timing between @sstwcw and my fix - we submitted our fixes less than 24 hours apart. I didn't think there would be another simultaneous fix for this 5 year o

[PATCH] D150403: [clang-format] Adjust braced list detection (try 2)

2023-05-15 Thread Galen Elias via Phabricator via cfe-commits
galenelias updated this revision to Diff 522245. galenelias edited the summary of this revision. galenelias added a comment. Addressed remaining review feedback. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150403/new/ https://reviews.llvm.org/D150403 Files: clang/lib/Format/Unwrapp

[PATCH] D150403: [clang-format] Adjust braced list detection (try 2)

2023-05-13 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. Please add the full stop to the code comment(s), and then mark review comments as done. You should also give credit to @sstwcw in the commit message/differential description for the token annotator tests. In D150403#4339530

[PATCH] D150403: [clang-format] Adjust braced list detection (try 2)

2023-05-12 Thread sstwcw via Phabricator via cfe-commits
sstwcw added a comment. > To this end, I am tracking the previous token type in a stack parallel > to LBraceStack to help scope this particular case. Maybe keep the description up to date with the code. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150403/new/ https://reviews.llvm.org/

[PATCH] D150403: [clang-format] Adjust braced list detection (try 2)

2023-05-12 Thread sstwcw via Phabricator via cfe-commits
sstwcw added a comment. I would suggest adding a link to the revision on the issue thread for your future bug fixes. This people like me don't try to fix what others have fixed. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150403/new/ https://reviews.llvm.org/D150403 ___

[PATCH] D150403: [clang-format] Adjust braced list detection (try 2)

2023-05-12 Thread Galen Elias via Phabricator via cfe-commits
galenelias added a comment. @HazardyKnusperkeks, thanks for the feedback. I added the TokenAnnotatorTests from @sstwcw's review. I also updated the design to use a single stack instead of the two parallel stacks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150403/new/ https://revi

[PATCH] D150403: [clang-format] Adjust braced list detection (try 2)

2023-05-12 Thread Galen Elias via Phabricator via cfe-commits
galenelias updated this revision to Diff 521828. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150403/new/ https://reviews.llvm.org/D150403 Files: clang/lib/Format/UnwrappedLineParser.cpp clang/unittests/Format/FormatTest.cpp clang/unittests/Format/TokenAnnotatorTest.cpp Index: cl

[PATCH] D150403: [clang-format] Adjust braced list detection (try 2)

2023-05-12 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. Shouldn't it have token annotator tests? Comment at: clang/lib/Format/UnwrappedLineParser.cpp:495-496 SmallVector LBraceStack; + // Track the previous token type corresponding to our lbraces // to help + // detect brace types + SmallVec

[PATCH] D150403: [clang-format] Adjust braced list detection (try 2)

2023-05-12 Thread Galen Elias via Phabricator via cfe-commits
galenelias added a subscriber: sstwcw. galenelias added a comment. Looks like @sstwcw also submitted a fix for the same issue, with a bit of a different approach: https://reviews.llvm.org/D150452 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150403

[PATCH] D150403: [clang-format] Adjust braced list detection (try 2)

2023-05-11 Thread Galen Elias via Phabricator via cfe-commits
galenelias created this revision. galenelias added reviewers: cpplearner, HazardyKnusperkeks, MyDeveloperDay. Herald added projects: All, clang, clang-format. Herald added a subscriber: cfe-commits. Herald added reviewers: rymiel, owenpan. galenelias requested review of this revision. This is a re