LGTM thanks for the patch

In principle, I think this is something that might help a lot of people as I've seen people asking for a better mechanism to be able to load a centrally stored .clang-format file.

LGTM, thanks once again for the C# patch!

This LGTM, thanks for the patch

Could you add an entry into the clang-format section of the clang release notes

is #44192 the correct bug?

LGTM

Thanks for the patch, really good to have others working on C# support. LGTM

I think generally speaking this looks ok, I'm ok with the extra clause, as long as it's clear what it's doing

Thanks for the patch.

But I don't know if this can be (easily) supported in a .clang-format file, since the style options are defined as (static) enums. I realize my proposal might be out of the scope of this patch, but I wanted to get some opinions from the community.

In D69764#1775835, @ilya wrote:

In D69764#1732235, @aaron.ballman wrote:

I like the functionality, but am slightly opposed to using "east/west" terminology -- that's not a ubiquitous phrase and it takes a bit of thinking before it makes sense. I think "left/right" is likely to be more universally understood.

Also, should this apply to other qualifiers like volatile or restrict? If so, the name ConstStyle should probably be CVQualifierStyle or something else.

+1.
In addition to volatile and restrict, in my organization we'd also be interested in applying this to address space qualifiers (custom keywords added in our downstream fork). Can this be generalized to accept a map of qualifier keywords and its desired orientation?

Drive by comment: (I have no vested interested other than seeing cool new checkers)

I recently hit this issue where I was clang-formatting generated code out of a source tree, which was then copied into a source tree. This failed my post build clang-format validation check and I couldn't understand why. it was only later I realised that unless I have the same .clang-format file locally when I clang-format the generated files or if I only clang-format them after I've copied them in will this work.

LGTM

A little advice as you fix each comment check the "Done" button.

As a consensus how about I drop Before/After and Keep Left/Right and West/East const support? (as no one is asking for Before/After?) can we handle having 2 options to aid language issues?

so now I think this is better because it's encased inside your TT_ObjCMethodExpr method which means it should only impact ObjectiveC

I guess this isn't something we could test with lit right?

I can tell from https://zed0.co.uk/clang-format-configurator/ that it's not possible to get it to the form you suggested, I'm not opposed to accepting this, I'm just not an ObjectiveC person so I haven't used clang-format on code like this, I just don't want to break others

Could you update the ClangStyleOption.rst (using the tool in docs/tools)

gentle ping

Can you rebase it and add a release note in docs/ReleaseNote.rst

I tend to agree, I'm not keen on the silent searching for the files.. this happens too much as it is, with Microsoft releasing VisualStudio with a clang-format.exe (v5.0), I suddenly find that binary is in my path, then all of a sudden all the 5.0 bugs we fixed since 5.0 reappear.

-update with missing files
-clang-format test

Can we get prop domain names as "*.group" gets blocked by lots of corporate firewalls.

• Rebase
• Add Release note
• Remove repeated lines cause by patch creation artefact

@Anteru thanks for the patch this got landed today, I got burnt by the 100755 mode change on the file, but thanks to @sylvestre.ledru it got resolved.

@sylvestre.ledru thanks for that.

In D6920#1748764, @sylvestre.ledru wrote:

Would be nice to add this to the release notes too ;)

Address review comments
Remove excessive newlines
Remove trailing whitespace

@Eugene.Zelenko I've noticed you are always giving excellent review feedback in clang-tidy especially around the documentation, I'd appreciate your eyes on finding the right level of documentation here.

Thank you for this patch. I'm sorry it took so long, I think this is a perfectly reasonable idea and helps cover the functionality that visual studio also supports. (which ultimately will help visual studio users embrace clang-format even more than they are already)

This LGTM, I think this could help more than people might at first realize. I love the idea that I can use clang-format in dry-run (-n) mode to now make line endings seem a compile error..

In D19031#1746705, @STL_MSFT wrote:

MSVC's STL currently uses CRLF (DOS) line endings, not LF (Unix). I wrote a validator, https://github.com/microsoft/STL/blob/58bb49d63d92e7a0346a05af29816aeea6b4cf0f/tools/validate/validate.cpp , to detect LF files, mixed line endings (LF and CRLF in the same file), damaged endings (CR only), and enforcing one newline at the end of every file, because clang-format doesn't currently handle those whitespace issues.

I suspect this patch might need a rebase, but I personally don't see anything wrong with it. I think this could really help.

LGTM

Nit: you should use a full context diff.

ok sounds good, @koalo do you have the tests to ensure AfterEnum=true and AllowShortEnumsOnASingleIne=true show on a single line? otherwise perhaps we are already good.

Sorry I'm reviewing very old revisions, @mxbOctasic are you still interested in a patch like this?

In D54628#1745645, @koalo wrote:

Yes, that is at least my understanding. I just rebased to master.

In my understanding, "short" means "put it on a single line if it fits considering the current maximum line length".

Thanks for rebasing, I think this is a good idea I'm just not sure about how the option presents itself, would you consider changing it?

Ironically the LLVM style guide says enumerators should be written out as you requested, but from what I can tell its not actually possible to get that format without this sort of change, do you agree?

In D54628#1744108, @koalo wrote:

Yes it does not. Therefore, I have followed the same pattern as the the other AllowShort* options.

It feels like it should somehow be following BreakBeforeBraces style but understand it might not

I'm thinking this is the same as BraceWrapping.AfterEnum, if you think your use case is covered would you consider Abandoning this revision so we know this functionality exists already, if not let work out what is missing