[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 marked an inline comment as not done. MarcusJohnson91 added inline comments. Comment at: clang/lib/Format/Format.cpp:586 IO.mapOptional("SortIncludes", Style.SortIncludes); IO.mapOptional("SortJavaStaticImport", Style.SortJavaStaticImport); IO.mapOp

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/Format.cpp:586 IO.mapOptional("SortIncludes", Style.SortIncludes); IO.mapOptional("SortJavaStaticImport", Style.SortJavaStaticImport); IO.mapOptional("SortUsingDeclarations", Style.SortUsingDeclarat

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/Format.cpp:746 + Expanded.BraceWrapping.SplitEmptyRecord = true; + Expanded.BraceWrapping.SplitEmptyNamespace = true; + Expanded.IndentExternBlock = FormatStyle::IEBS_AfterExternBlock; I didn't

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

sylvestre.ledru added a comment. @MarcusJohnson91 I know it is confusing but we don't use the Mozilla coding style. We are using the Google style. You can use the code snippet that I provided here: https://bugs.llvm.org/show_bug.cgi?id=47589 it is easy to reproduce even without argument or conf

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 added a comment. @sylvestre.ledru After looking more closely at the issue, it seems you're having an issue with Mozilla's comment alignment option. you want the comments to be aligned, and it appears Clang11 no longer has that option set for Mozilla's style is what you're sayin

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 updated this revision to Diff 293240. MarcusJohnson91 edited the summary of this revision. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 Files: clang/lib/Format/Format.cpp Index: clang/lib/Format/Format.cpp ===

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 added a comment. In D75791#2283961 , @sylvestre.ledru wrote: > Any chance this changes could have caused this regression > https://bugs.llvm.org/show_bug.cgi?id=47589 ? I don't think so, but I can double check the style defaults for the

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

sylvestre.ledru added a comment. Any chance this changes could have caused this regression https://bugs.llvm.org/show_bug.cgi?id=47589 ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MyDeveloperDay added inline comments. Comment at: clang/include/clang/Format/Format.h:1531 +/// \endcode +/// +/// \code @MarcusJohnson91 I had to make a couple of minor changes before committing 1) it needed rebasing (because of changes from this m

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

This revision was automatically updated to reflect the committed changes. Closed by commit rG6ef45b0426a8: [clang-format] Added new option IndentExternBlock (authored by MyDeveloperDay). Changed prior to commit: https://reviews.llvm.org/D75791?vs=264916&id=265337#toc Repository: rG LLVM Gith

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MyDeveloperDay added a comment. In D75791#2045986 , @MarcusJohnson91 wrote: > In D75791#2044492 , @MyDeveloperDay > wrote: > > > If you want me to land this for you, I'd feel more comfortable landing it > > if: >

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 added a comment. In D75791#2044492 , @MyDeveloperDay wrote: > If you want me to land this for you, I'd feel more comfortable landing it if: > > a) We can land D80214: [clang-format] Set of unit test to begin to validate > that we don't ch

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MyDeveloperDay added a comment. If you want me to land this for you, I'd feel more comfortable landing it if: a) We can land D80214: [clang-format] Set of unit test to begin to validate that we don't change defaults first b) The Mozilla team have tested the impa

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

Abpostelnicu added a comment. Let's first see we don't break anything on `mozilla`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lis

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 updated this revision to Diff 264916. MarcusJohnson91 added a comment. Format.h: indented the ``AfterExternBlock: true`` example code snippet with 4 spaces like the Indent option so it's more visible and matches. I think it's perfect now. CHANGES SINCE LAST ACTION https://rev

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 updated this revision to Diff 264909. MarcusJohnson91 added a comment. Just fixed the formatting of the ReleaseNotes.rst file, the extern blocks were slightly askew, and it might've made it a bit confusing CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https:/

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 updated this revision to Diff 264911. MarcusJohnson91 added a comment. Made the IndentExternBlockStyle enum comments a bit clearer, and regenerated the .rst file CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 Files: clang/doc

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. I think this LGTM now... CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MyDeveloperDay added a comment. This is what I was thinking about D80214: [clang-format] Set of unit test to begin to validate that we don't change defaults CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 __

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 marked 5 inline comments as done. MarcusJohnson91 added inline comments. Comment at: clang/lib/Format/Format.cpp:812 true, true}; + LLVMStyle.IndentExternBlock = FormatStyle::IEBS_AfterExternBlock; LLVMStyle.BreakAfterJavaFieldAn

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 added a comment. In D75791#2043758 , @MyDeveloperDay wrote: > Sorry to "go around the houses" but we'll get there in the end...I think we > are close I think we're close too. Your other comment was interesting, about testing the styles

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MyDeveloperDay added a comment. Sorry to "go around the houses" but we'll get there in the end...I think we are close CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 ___ cfe-commits mailing list cfe

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MyDeveloperDay added a comment. All this confusion over the defaults is because we don't have the unit tests to check the default of each style. If we had that we could have a before/after conversation more easily Nit: also please mark comments done once you have addressed them.

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MyDeveloperDay added a comment. > btw, in .BraceWrapping = {true, false); blocks, AfterExternBlock is the 9th > option, I checked the BraceWrapping enum. This will become clearer I hope when I land D79325: [clang-format] [PR42164] Add Option to Break before While

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 updated this revision to Diff 264839. MarcusJohnson91 added a comment. Ok, I've removed the inherited ones, and also removed the times I was setting a style when there wasn't one before. also I moved the `IEBS_AfterExternBlock` line to right underneath the `BraceWrapping.AfterEx

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 marked 4 inline comments as done. MarcusJohnson91 added inline comments. Comment at: clang/lib/Format/Format.cpp:714 case FormatStyle::BS_Mozilla: +Expanded.IndentExternBlock = FormatStyle::IEBS_AfterExternBlock; Expanded.BraceWrapping.AfterClass = tru

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MyDeveloperDay added subscribers: Abpostelnicu, sylvestre.ledru. MyDeveloperDay added a comment. This is totally fine, now I'm just concerned by the choice of defaults, I really don't know if we want to change the defaults for all the styles, I don't want to break all those people using it One

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 added a comment. As for crashes, none of them seem relevant; I'm on MacOS, the windows ABI crash seems especially irrelevent. opt crashed, there were no arguments, and abort() was called. llvm-lto2 crashed, not-prevailing.ll.tmp1-3.bc was the cause llvm-dwarfdump crashed, argum

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 updated this revision to Diff 264763. MarcusJohnson91 added a comment. Added the style initializers, moved IEBS_AfterExternBlock to be the first enum value so that it's zero, that way the bool logic works. Regenerated the docs as well, and also clang-formatting the files I've tou

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 added a comment. I've initialized all styles to either AfterExternBlock, if there was a BraceWrapping block, or NoIndent if there wasn't. re-running my tests locally. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 __

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 added a comment. I've got the indenting to work manually now as well, the issue was you need to have `BreakBeforeBraces: Custom` in the inline style for it to pick up BraceWrapping.AfterExternBlock's value. Now I'm working on the automated tests, thanks for the tip about initial

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MyDeveloperDay added a comment. I think it needs to be: `LLVMStyle.IndentExternBlock = FormatStyle::IEBS_NoIndent;` I'm wondering if the GNU style also needs `Expanded.IndentExternBlock = FormatStyle::IEBS_Indent;` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://revi

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MyDeveloperDay added a comment. > if thats true how didn't the automated tests catch it? If you are not in the premerge testing group the unit tests won't be run. (which is why I added you) When I run the gtest it fails, I cannot see why.. but one suggestion I have is to drop the whole true/fa

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MyDeveloperDay added a comment. ok it crashes because you are not initializing IndentExternBlock in the getLLVMStyle() function LLVMStyle.IndentExternBlock = FormatStyle::IEBS_NoIndent; You cannot leave it uninitialized, now what the correct value is in my view may be an issue CHANGES SINCE

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MyDeveloperDay added a comment. Sorry forgot to submit the comment issue, seems like you worked that bit out already CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 ___ cfe-commits mailing list cfe-

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MyDeveloperDay added a comment. I've added you to https://reviews.llvm.org/project/view/78/ Comment at: clang/include/clang/Format/Format.h:1555 + IndentExternBlockStyle IndentExternBlock; + /// A vector of prefixes ordered by the desired groups for Java imports. --

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 updated this revision to Diff 264597. MarcusJohnson91 added a comment. Fixed the generation of ReleaseNotes.rst CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 Files: clang/docs/ClangFormatStyleOptions.rst clang/docs/ReleaseN

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 added a comment. In D75791#2040665 , @MyDeveloperDay wrote: > > Something is not quite right here, this text isn't ending up in the > ClangFormatStyleOptions.rst You're right, I didn't catch that before, turns out having a comment be

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MyDeveloperDay added inline comments. Comment at: clang/include/clang/Format/Format.h:1555 + IndentExternBlockStyle IndentExternBlock; + /// A vector of prefixes ordered by the desired groups for Java imports. Something is not quie right here, this text isn't

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MyDeveloperDay added a comment. Please clang-format the patch CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/ma

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MyDeveloperDay added a comment. In D75791#2040558 , @MarcusJohnson91 wrote: > In D75791#2040532 , @MyDeveloperDay > wrote: > > > LGTM > > > So what's the next step? I've never committed to LLVM before. In my min

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 added a comment. In D75791#2040532 , @MyDeveloperDay wrote: > LGTM So what's the next step? I've never committed to LLVM before. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 _

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MyDeveloperDay added a comment. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commit

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 marked an inline comment as done. MarcusJohnson91 added a comment. Removed the lowercase Noindent case, that was a last minute addition I thought might make it a tad easier to work with, but you're right I didn't even test it, and honestly adding that complexity is just pointless

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 updated this revision to Diff 264462. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 Files: clang/docs/ClangFormatStyleOptions.rst clang/docs/ReleaseNotes.rst clang/include/clang/Format/Format.h clang/lib/Format/Format.cpp

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. LGTM just drop the `Noindent` it will encourage inconsistency people can read the manual (plus you don't check it in the tests) Comment at: clang/lib/Format

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 updated this revision to Diff 264439. MarcusJohnson91 added a comment. Removed forgotten comment from control logic of UnwrappedLineParser CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 Files: clang/docs/ClangFormatStyleOption

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 updated this revision to Diff 264438. MarcusJohnson91 edited the summary of this revision. MarcusJohnson91 added a comment. Did everything you asked and did a littl bit of my own cleanup as well. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.ll

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 marked 3 inline comments as done. MarcusJohnson91 added a comment. I've fixed all of your comments as well as fixed the tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 ___ cfe-

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MyDeveloperDay added a comment. You need to regenerate the ClangFormatStyleOption.rst file using docs/tools/dump_format_style.py Comment at: clang/include/clang/Format/Format.h:965 bool AfterExternBlock; +enum ExternBlock { + /// Break extern blocks before the cu

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 updated this revision to Diff 258612. MarcusJohnson91 edited the summary of this revision. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Format/Format.h clang/lib/Forma

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 added a comment. @MyDeveloperDay > but I'm also constantly surprised by how many of the enumeration cases > started out as booleans only later to have to be converted to enums. The more > I think about this the more I think the problem can probably be dealt with > better by mak

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MyDeveloperDay added a comment. > Ok, I've got an idea to deprecate the AfterExternBlock option and map it to a > new option Really? can't you just model IndentExternBlock as an enum? then say bool shouldIndent = Style.IndentExternBlock==Indented || ( Aft

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 added a comment. I agree that changing formatting randomly isn't a good idea, and I think converting AfterExternBlock to an enum is the way to go, but I'm just not sure on how it should be implemented. Ok, I've got an idea to deprecate the AfterExternBlock option and map it to a

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MyDeveloperDay added a comment. I have no concerns with this patch other than the consideration of the defaults. My concern is whatever our choice we will generate churn on existing code where the .clang-format file has AfterExternBlock = true (or its true in inbuilt BasedOnStyle type) https:

Re: [PATCH] D75791: [clang-format] Added new option IndentExternBlock

I’m wondering if theres a way to tell if an option has been set in the clang-format config file. If IndentExternBlock is not set, we should default to the old behavior. As for your suggestion of setting IndentExternBlock to BraceWrapping.AfterExternBlock or negating it, neither way works with t

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MyDeveloperDay added a comment. Could the default be `Style.IndentExternBlock = Style.BraceWrapping.AfterExternBlock` Then it would match the prior behaviour off AddLevel being `true` when AfterExternBlock is `true` extern "C" { int a; } extern "C" { int a; } CHANGES SINC

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 updated this revision to Diff 252730. MarcusJohnson91 added a comment. Implemented the suggestion to break the test strings down into smaller pieces CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 Files: clang/docs/ReleaseNotes

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 marked 2 inline comments as done. MarcusJohnson91 added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:2497 Style); } MyDeveloperDay wrote: > my assumption is this test is using `Style.IndentExternBlock =fals

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:2440 TEST_F(FormatTest, FormatsExternC) { - verifyFormat("extern \"C\" {\nint a;"); - verifyFormat("extern \"C\" {}"); + verifyFormat("extern \"C\" {\nint a; /*2.1*/"); + verifyFormat("ex

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 updated this revision to Diff 252466. MarcusJohnson91 marked 4 inline comments as done. MarcusJohnson91 added a comment. Implemented @MyDeveloperDay's suggestion to simplify the if/else statements. Removed all the test changes except one: That's because the BraceWrapping.AfterExt

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 marked an inline comment as done. MarcusJohnson91 added a comment. Restored all the test function names to foo(). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 ___ cfe-commits maili

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:31 +class FormatTest +: public ::testing::Test { // FormatTest is a Fixture, data is reused protected: MyDeveloperDay wrote: > is this comment necessary? Removed the com

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MyDeveloperDay added a comment. > Not sure if tagging is considered rude, I figure that @MyDeveloperDay's > notification fell off your radar. Definitely not rude from my perspective...perfectly happy to come and look and be tagged to get my attention. If I'm ignoring its, its only becuase I'm b

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:2468 "}"); - verifyFormat("extern \"C\" int foo() {}"); - verifyFormat("extern \"C\" int foo();"); - verifyFormat("extern \"C\" int foo() {\n" + verifyFormat("extern \"C\"

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1088 if (FormatTok->Tok.is(tok::l_brace)) { -if (Style.BraceWrapping.AfterExternBlock) { - parseBlock(/*MustBeDeclaration=*/true); -} else { +if (Sty

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MyDeveloperDay added a comment. @MarcusJohnson91 To get a review past its better that you mark the comments as done so the reviewers know when the comments have been addressed and not missed. (this is important as the number of comments grows) Developers need to explain why they haven't changed

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 added a comment. Rebased on Master again, recompiling and re-running all the tests. I'll update this comment when it passes, or create a new diff if it doesn't but nothing had to be changed so it'll probably work. @krasimir I noticed that you've been active recently, can you rev

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 updated this revision to Diff 251846. MarcusJohnson91 added a comment. Rebased on master CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Format/Format.h clang/lib/Forma

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 added a comment. Bump CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commi

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 updated this revision to Diff 250354. MarcusJohnson91 added a comment. Fixed Format.h comments, and rebased on master. it's perfect on my end now. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 Files: clang/docs/ReleaseNotes.

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 updated this revision to Diff 249954. MarcusJohnson91 added a comment. Rebased CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Format/Format.h clang/lib/Format/Format.c

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 updated this revision to Diff 249838. MarcusJohnson91 added a comment. New squashed commit with all changes present CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Format

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 added a comment. In D75791#1917837 , @MyDeveloperDay wrote: > your patch seems to be missing the other files Which other files? I made a new commit and did the full context diff, now sure why it's only showing the documentation update.

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MyDeveloperDay added a comment. your patch seems to be missing the other files CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.ll

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 updated this revision to Diff 248975. MarcusJohnson91 added a comment. Updated the release notes CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 Files: clang/docs/ReleaseNotes.rst Index: clang/docs/ReleaseNotes.rst ==

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 updated this revision to Diff 248974. MarcusJohnson91 added a comment. Removed the debugging comments I added to FormatTest.cpp CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 Files: clang/include/clang/Format/Format.h clang/

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 added a comment. In D75791#1911133 , @MyDeveloperDay wrote: > you need documentation and release note changes too The comments were only for testing, I'll remove them. The tests had to change because the behavior has changed slightly.

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MyDeveloperDay added a comment. you need documentation and release note changes too Comment at: clang/unittests/Format/FormatTest.cpp:31 +class FormatTest +: public ::testing::Test { // FormatTest is a Fixture, data is reused protected: is this comment nec

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1088 if (FormatTok->Tok.is(tok::l_brace)) { -if (Style.BraceWrapping.AfterExternBlock) { - parseBlock(/*MustBeDeclaration=*/true); -} else { +if (Sty

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MarcusJohnson91 created this revision. MarcusJohnson91 added a reviewer: cfe-commits. Herald added a project: clang. and refactored the BraceWrapping.AfterExternBlock code so that it only deals with wrapping the brace after an extern block. Repository: rG LLVM Github Monorepo https://reviews