[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

Sedeniono updated this revision to Diff 539583. Sedeniono added a comment. @owenpan Oh wow, you are right. Phabricator shows that there are two separate commits, under "Revision Contents" > "Commits", and their commit messages. But apparently there is no way to download some proper *.patch file

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

owenpan added a comment. In D151047#4449996 , @Sedeniono wrote: > As suggested by @owenpan, I split the commits differently. The first commit > now contains only the fix + corresponding tests. The second commit contains > refactorings/asserts. How do I

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

Sedeniono added a comment. If everyone is ok with the changes, could someone commit it for me since I do not have commit rights? Thanks! `Sedenion <39583823+sedeni...@users.noreply.github.com>` Note: I cannot really make sense of the error reported by the pre merge checks

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

Sedeniono updated this revision to Diff 534688. Sedeniono added a comment. As suggested by @owenpan, split the commits differently. My previous statement, that the whole fix consists of a revert of D129064 was not entirely correct: The fix reintroduces the `re

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

owenpan added a comment. In D151047#4396727 , @Sedeniono wrote: > So, the new patch basically just reverts the original fix of > https://reviews.llvm.org/D129064 (i.e. it re-introduces the `resize()` in > `LevelIndentTracker::nextLine()`). The other cha

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

Sedeniono marked an inline comment as done. Sedeniono added a comment. @owenpan , @MyDeveloperDay Any opinion on the latest changes? Otherwise, since I do not have commit rights, someone needs to commit the changes to main (my name and mail: `Sedenion <39583823+sedeni...@users.noreply.github.co

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

Sedeniono marked an inline comment as done. Sedeniono added inline comments. Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:80 + // directive, since these do not terminate a code block. + if (!Line.InPPDirective && Line.Level < IndentForLevel.size()) +I

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

Sedeniono updated this revision to Diff 530627. Sedeniono marked an inline comment as done. Sedeniono added a comment. Updated diff: Captured the intention of "going to lower levels" via a debug assert in nextLine(). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:80 + // directive, since these do not terminate a code block. + if (!Line.InPPDi

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

Sedeniono marked an inline comment as done. Sedeniono added inline comments. Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:169-170 /// The indent in characters for each level. + /// It remembers the indent of previous lines (that are not PP directives) of + /// e

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

Sedeniono updated this revision to Diff 529631. Sedeniono added a comment. Next try at getting all changes to phabricator. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151047/new/ https://reviews.llvm.org/D151047 Files: clang/lib/Format/Unwrapp

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

Sedeniono updated this revision to Diff 529630. Sedeniono added a comment. Reformatted comment, and submitting it via `arc diff --update` now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151047/new/ https://reviews.llvm.org/D151047 Files: clan

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

barannikov88 added inline comments. Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:169-170 /// The indent in characters for each level. + /// It remembers the indent of previous lines (that are not PP directives) of + /// equal or lower levels. This is used to ali

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

Sedeniono marked an inline comment as done. Sedeniono added inline comments. Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:169-170 /// The indent in characters for each level. + /// It remembers the indent of previous lines (that are not PP directives) of + /// e

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

Sedeniono marked an inline comment as done. Sedeniono added inline comments. Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:80 + // directive, since these do not terminate a code block. + if (!Line.InPPDirective && Line.Level < IndentForLevel.size()) +I

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

Sedeniono updated this revision to Diff 529517. Sedeniono added a comment. Reformatted comment as requested. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151047/new/ https://reviews.llvm.org/D151047 Files: clang/lib/Format/UnwrappedLineFormatter.cpp Index: clang/lib/Format/Unwrapp

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:80 + // directive, since these do not terminate a code block. + if (!Line.InPPDirective && Line.Level < IndentForLevel.size()) +IndentForLevel.resize(Line.Level + 1

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

Sedeniono added a comment. @barannikov88 ok, thanks. I just did that. The latest diff looks right. I wondered because phabricator suddenly showed changes in ocaml files, which I certainly did not want. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

Sedeniono updated this revision to Diff 528547. Sedeniono added a comment. Next try at getting the 2nd attempt into review. This is again the complete fix, since the previous revision got reverted. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1510

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

barannikov88 added a comment. In D151047#4396739 , @Sedeniono wrote: > Oh damn, I executed `arc diff --update D151047` on the main branch instead of > my own local branch... Sorry. > Can I somehow revert that? Or should I simply use the "abandon review"

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

Sedeniono added a comment. Oh damn, I executed `arc diff --update D151047` on the main branch instead of my own local branch... Sorry. Can I somehow revert that? Or should I simply use the "abandon review" action and create a entirely new one? Repository: rG LLVM Github Monorepo CHANGES SIN

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

Sedeniono updated this revision to Diff 528531. Sedeniono added a comment. Herald added subscribers: llvm-commits, pcwang-thead. Herald added a project: LLVM. Fixes github issues #59178, #58464 and #62799. The problem was that the LevelIndentTracker remembered the indentation level of pre

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

Sedeniono reopened this revision. Sedeniono added a comment. This revision is now accepted and ready to land. Ok, here is the 2nd attempt at fixing the issue. The patch is based on the current main branch. Regarding `LineJoiner::join()`: Turns out, not setting `A.Level` but reverting the origin

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

barannikov88 added a comment. In D151047#4369742 , @Sedeniono wrote: > To create a new fix, do I understand the guide > correctly that > I basically execute `arc diff --verbatim` and mention

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

HazardyKnusperkeks added a comment. In D151047#4369742 , @Sedeniono wrote: > To create a new fix, do I understand the guide > correctly that > I basically execute `arc diff --verbatim` and m

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

Sedeniono added a comment. Heh, ok, so I wasn't that naive then to not run the tests of everything :-) I had a look at the issue. The ClangRenameTests first do some replacements, and then call formatAndApplyReplacements()

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

HazardyKnusperkeks added a comment. In D151047#4369503 , @MyDeveloperDay wrote: > I think we need to extract the context of the test from RenameTests to ensure > we have it covered here. I don't personally normally run the entire LLVM > suite. Dito, a

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

MyDeveloperDay added a comment. I think we need to extract the context of the test from RenameTests to ensure we have it covered here. I don't personally normally run the entire LLVM suite. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151047/new/

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

Sedeniono added a comment. Ok, sorry, the test failures are on me. There is a lesson learned for me: I naively ran only the formatter unit tests, not the whole test suite of llvm. I somehow assumed that my code changes only affected clang-format, since the UnwrappedLineFormatter is in the "Form

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

dyung added a comment. Hi @owenpan and @Sedeniono, this change is causing test failures across many bots, can you take a look and fix or revert if you need time to investigate? Failing bots: - https://lab.llvm.org/buildbot/#/builders/139/builds/41339 - https://lab.llvm.org/buildbot/#/builders/2

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG72ab89e3197c: [clang-format] Fix indentation for selective formatting (authored by Sedeniono, committed by owenpan). Repository: rG LLVM Github Mo

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

HazardyKnusperkeks added a comment. In D151047#4363127 , @owenpan wrote: > In D151047#4361205 , @Sedeniono > wrote: > >> Also, should it be merged into the LLVM 16.x branch? > > +1. @MyDeveloperDay and @HazardyKn

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

Sedeniono added a comment. @owenpan Here is the name and mail in the <> format: `Sedenion <39583823+sedeni...@users.noreply.github.com>` @HazardyKnusperkeks Ok, I will remember it for the next one. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1510

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

owenpan accepted this revision. owenpan added a comment. In D151047#4361205 , @Sedeniono wrote: > Regarding name and mail: If you use "Sedenion" as name and > "39583823+sedeni...@users.noreply.github.com" as mail, I guess github will > attribute the com

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. Just a side note, please link to commits, or even better the reviews here when talking about old commits. I found it through the github issue, but I think the other way around is better. I was interested in the commit

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

Sedeniono added a comment. @MyDeveloperDay Thanks for the positive review. Regarding https://github.com/llvm/llvm-project/issues/56352 and the Beyoncé Rule, the original fix for the crash actually added a test (see the changes made at that time

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. Thank you for the indepth explaination in https://github.com/llvm/llvm-project/issues/59178, that was really helpful for me trying to understand what the problem was. I thik y

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

Sedeniono added a comment. The build failures seem to be unrelated to my changes. The builds of other reviews also show them, and the error messages point to things I haven't changed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151047/new/ http

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

Sedeniono created this revision. Sedeniono added a reviewer: curdeius. Herald added projects: All, clang, clang-format. Herald added a subscriber: cfe-commits. Herald added reviewers: rymiel, HazardyKnusperkeks, owenpan, MyDeveloperDay. Sedeniono requested review of this revision. Fixes github iss