[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rL356613: [clang-format] BeforeHash added to IndentPPDirectives (authored by paulhoad, committed by ). Herald added a

[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

to-miz updated this revision to Diff 191540. to-miz added a comment. Made a new diff on the git repository, since the docs now recommend using git. Rebased and created a new diff with `git show HEAD -U99`. The patch applies cleanly on windows after `git reset --hard` so I hope there

[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

MyDeveloperDay added a comment. In D52150#1436423 , @to-miz wrote: > I checked out the git repository and applied the patch to head and reran all > tests. > The diff produced by git is basically the same, should I upload the new diff? I think you

[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

to-miz added a comment. I checked out the git repository and applied the patch to head and reran all tests. The diff produced by git is basically the same, should I upload the new diff? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52150/new/ https://reviews.llvm.org/D52150

[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

kadircet added a comment. In D52150#1436278 , @to-miz wrote: > Is it because the diff was made with the svn repository? It is for the newest > revision. > Should I make a new diff with the git repo instead? > Or is it not because of the diff, but

[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

to-miz added a comment. Is it because the diff was made with the svn repository? It is for the newest revision. Should I make a new diff with the git repo instead? Or is it not because of the diff, but something in phabricator instead? Sorry this is my first patch. CHANGES SINCE LAST ACTION

[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

kadircet added a comment. Could you do a rebase? I've tried to land it but there are too many merge conflicts currently. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52150/new/ https://reviews.llvm.org/D52150 ___ cfe-commits mailing list

[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

to-miz added a comment. I don't have commit access. It would be nice if someone could commit it in my place. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52150/new/ https://reviews.llvm.org/D52150 ___ cfe-commits mailing list

[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

MyDeveloperDay added a comment. @to-miz do you have permission to commit? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52150/new/ https://reviews.llvm.org/D52150 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

klimek accepted this revision. klimek added a comment. Given this doesn't add an extra option (but only an extra enum) and is fairly straight forward, LG. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52150/new/ https://reviews.llvm.org/D52150

[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. Thank you for helping to address one of the many clang-format issues from bugzilla, I'm not the code owner here but this looks good to me. If we want to address the many issues we need to be able to move forwards. CHANGES

[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

to-miz updated this revision to Diff 189048. to-miz added a comment. Added an entry to ReleaseNotes.rst about the new option. This diff doesn't contain unrelated formatting changes due to running clang-format on some source files that apparently weren't formatted before. CHANGES SINCE LAST

[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

MyDeveloperDay added a comment. While we wait for code review I've landed this currently unaccepted clang-format revision into https://github.com/mydeveloperday/clang-experimental/releases for those wishing to try/test or provide feedback CHANGES SINCE LAST ACTION

[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

MyDeveloperDay marked an inline comment as done. MyDeveloperDay added inline comments. Comment at: lib/Format/UnwrappedLineFormatter.cpp:15 #include #define DEBUG_TYPE "format-formatter" Nice! its the irony of clang-format code is that its not clang

[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

MyDeveloperDay added a comment. Could you add some text into the docs/ReleaseNotes.rst to say you are adding a new option take a look at https://clang.llvm.org/extra/ReleaseNotes.html#improvements-to-clang-tidy for an example CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52150/new/

[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

to-miz added a comment. I understand that adding new options adds costs to the development of clang-format, but there was a feature request and there is interest for this option. As for the cost to the development, there already exists PPDIS_AfterHash. All I did was make the changes introduced

[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

geleji added a comment. I would find this useful too. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52150/new/ https://reviews.llvm.org/D52150 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

ufna added a comment. @krasimir BeforeHash style is widely used for Unreal Engine 4 based projects development. I was looking for this option too. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52150/new/ https://reviews.llvm.org/D52150

[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

krasimir requested changes to this revision. krasimir added a comment. This revision now requires changes to proceed. As per https://clang.llvm.org/docs/ClangFormatStyleOptions.html#adding-additional-style-options, could you please provide more info about the need for this option first?

[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

to-miz updated this revision to Diff 165700. to-miz added a comment. PPBranchLevel can be negative, while Line->Level is unsigned. Added check to ensure that PPBranchLevel is positive before adding to Line->Level. https://reviews.llvm.org/D52150 Files: docs/ClangFormatStyleOptions.rst

[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

to-miz created this revision. to-miz added reviewers: djasper, klimek, krasimir, sammccall, mprobst, Nicola. Herald added a subscriber: cfe-commits. The option BeforeHash added to IndentPPDirectives. Fixes Bug 36019. Repository: rC Clang https://reviews.llvm.org/D52150 Files: