- User Since
- Aug 18 2015, 2:16 AM (230 w, 4 d)
Thu, Jan 16
LGTM thanks for the patch
Wed, Jan 15
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.
Thu, Jan 9
LGTM, thanks once again for the C# patch!
Wed, Jan 8
This LGTM, thanks for the patch
Tue, Jan 7
Could you add an entry into the clang-format section of the clang release notes
Sun, Jan 5
is #44192 the correct bug?
Sat, Jan 4
Thanks for the patch, really good to have others working on C# support. LGTM
Mon, Dec 30
I think generally speaking this looks ok, I'm ok with the extra clause, as long as it's clear what it's doing
Sat, Dec 21
Fri, Dec 20
Dec 19 2019
Thanks for the patch.
Dec 14 2019
Dec 11 2019
Dec 10 2019
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.
Dec 9 2019
Dec 6 2019
Dec 5 2019
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.
Dec 4 2019
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
Dec 3 2019
I guess this isn't something we could test with lit right?
Dec 2 2019
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)
Nov 30 2019
Nov 25 2019
Nov 20 2019
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.
Nov 19 2019
-update with missing files
Can we get prop domain names as "*.group" gets blocked by lots of corporate firewalls.
- Add Release note
- Remove repeated lines cause by patch creation artefact
Nov 18 2019
Nov 16 2019
@sylvestre.ledru thanks for that.
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.
Nov 15 2019
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..
Nov 14 2019
I suspect this patch might need a rebase, but I personally don't see anything wrong with it. I think this could really help.
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?
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?
Nov 13 2019
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