Thanks for explaining! I visited the his profile. It is really confusing.

In D94891#2505736, @ChuanqiXu wrote:

@231084fg may I ask why you remove the reviews for the patch?

Similarly I see users having to define everything just to turn one thing off

I think this is LGTM, however..

What I can't easily tell from the tests is if you are overriding any styles defined in the parent with a local style.

I think the revision whilst it does what is needed to structs doesn't address the many other times this forms appear. I think we need something a little more extensive. It can't just be when a line starts with struct

Please address the "not done" comment (regarding the sorting), but other than that its LGTM

Sorry but due to the following issue raised by @RatTac , I'm reverting this prior to the LLVM 12 branch out so I can work on it further.

Thank you for changing the dump_style to make this clearer

Thie LGTM, just to check a few nits

My assumption is that you want to stick with the minimum and maximum is that correct?

I think I would remove the code examples from the "AlignConsecutive style" to avoid confusion (that would be the first change)

LGTM

This LGTM

@RatTac Would you please consider copying the comment onto the review D92753: [clang-format] Add IndentPragma style to eliminate common clang-format off scenario and not the commit, I have a reply but don't want to write it here.

Thank you for submitting this patch?

This LGTM, I'm not sure if others have any further comments

I'm sorry, but now you are missing the files from the review, I think you diffing against your own commits and not commit that are in the repo. This makes the review very difficult to understand.

we don't commit with failing tests so lets understand why it fails.

by and large, this looks pretty good to me..

You need to have these conversations by adding new unit tests that prove your point, I highly doubt I'll personally be willing to accept any revision without more unit tests than this one line change

I think we are missing some clarity in this bug as to what the actual problem is, I do agree the test looks wrong,

ok so this looks ok, but before we commit can we have a discussion about why you think it fails for the comment case?

Take the following example:

I think if you think you have a bug that you log it in the bug tracker and we track it with this issue.

Address review comments

@mitchell-stellar do you plan to bring this back?

I like what you are suggesting, do you think BasedOnStyle: File is the best terminology as the term File is used elsewhere to mean read from the .clang_format file, how about

Addressing additional usecase found issues in C# tests too

I'm a little confused why this is needed as clang-format always read up the directory tree until it see a .clang-format file, perhaps I don't quite understand the use case. Can't you just have a different .clang-format in the subdirectory?

Thank you for this patch, a few process issues.

This is the closest I could find thus far http://bitsavers.informatik.uni-stuttgart.de/pdf/chromatics/CGC_7900_C_Programmers_Manual_Mar82.pdf

I don't have any major issues with this other than it makes Format.h a bit more ugly. (there are more ugly and harder to understand pieces of code than this change!)

We should probably check the docs/tools/dump_format_style.py still works

LGTM

In D93626#2467745, @curdeius wrote:

LGTM now. I tried to find other cases where this change may change the behaviour but couldn't. Have you tried applying to some bigger repo and see what you get? The best would be a repo with SpaceAfterCStyleCast=true.

remove = and ~ cases

remove unrelated change

Thanks for making the changes, this LGTM

With my maintainer-of-Support/JSON hat on, I don't like the idea of these features being shoehorned into the library to make clang-format an incrementally more featureful JSON formatter. They're likely to lead to a lot of conceptual+implementation+API complexity in a library that solves its primary use cases quite well and simply.

Maybe @JVApen can help as they logged https://bugs.llvm.org/show_bug.cgi?id=48539

In D93528#2462723, @curdeius wrote:

Why not using clang-format's support for JavaScript? It seems more complicated now, but it will probably allow to more easily change/add style options. WDYT?
And what happens when reformatting only a part of a JSON? Should there be some tests for that?

LGTM