fodinabor updated this revision to Diff 292781.
fodinabor marked an inline comment as done.
fodinabor added a comment.
Fix the nit :)
Anyone with access may commit this now (ideally ofc someone with a final
opinion on the clang-format part).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE
grimar accepted this revision.
grimar added a comment.
This revision is now accepted and ready to land.
LGTM. It worth wainting for a second approvement and/or other comments to
verify that people are happy with doing this for `clang-format`.
Comment at: clang/unittests/Format
fodinabor added a comment.
All comments should be adressed now.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86137/new/
https://reviews.llvm.org/D86137
___
cfe-commits mailing list
cfe-commits@lists.llv
fodinabor updated this revision to Diff 291826.
fodinabor marked 9 inline comments as done.
fodinabor added a comment.
Unit test cleanups.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86137/new/
https://reviews.llvm.org/D86137
Files:
clang/docs
grimar added inline comments.
Comment at: clang/unittests/Format/FormatTest.cpp:16079
+ auto Style9 = getStyle("file", "/d/.clang-format", "LLVM", "", &FS, true);
+ ASSERT_TRUE((bool)Style9);
}
It looks like it is very similar to "Test 7:" and can be a part of
fodinabor updated this revision to Diff 291405.
fodinabor added a comment.
Address review comments, copy the help text into docs and add some basic unit
tests.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86137/new/
https://reviews.llvm.org/D8613
grimar added inline comments.
Comment at: llvm/lib/Support/YAMLTraits.cpp:204
if (!is_contained(MN->ValidKeys, NN.first())) {
- setError(NN.second.get(), Twine("unknown key '") + NN.first() + "'");
- break;
+ auto ReportNode = NN.second.get();
+ auto Repo
fodinabor updated this revision to Diff 290461.
fodinabor added a comment.
Remove test entry form .clang-format :)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86137/new/
https://reviews.llvm.org/D86137
Files:
clang/include/clang/Format/Format.
fodinabor updated this revision to Diff 290460.
fodinabor marked 9 inline comments as done.
fodinabor added a comment.
Incorporating review comments:
- renaming option to -Wno-error=unknown and adding warning in description
- emit warnings instead of fully ignoring the issues
Documentation and u
JakeMerdichAMD added a comment.
I can see the use of this, but I am also wary that ignoring style options will
lead to people producing different results on different versions of
clang-format. This is both because having set-or-unset an option will naturally
lead to different code and also that
grimar added a comment.
I am not familar with `clang-format`, but have a few comments inlined about the
rest.
I think the new `setIgnoreUnknown` YAMLlib API is probably OK generally.
I'd perhaps call it differently, e.g. `setAllowUnknownKeys` though.
Also, I think you need to add a unit testing
fodinabor added a subscriber: grimar.
fodinabor added a comment.
Thank you so far for the feedback!
maybe you can give further guidance on the comments on the comments :)
As of the git history it seems that @grimar did some work on YAML error
handling..
Comment at: clang/tool
MyDeveloperDay added subscribers: sammccall, JDevlieghere, aaron.ballman.
MyDeveloperDay added a comment.
> Regarding not touching the LLVM support library: I'd love to find a way, but
> as clang-format uses the >> operator
We need to find some reviewers who look after a wider area of LLVM, if y
fodinabor added a comment.
I see the possible issue with the possible version mismatches and that is why
I'd make people opt-in for this option, to still being able to format their
files (e.g. if using out-dated built-in versions like in Visual Studio - I know
you can specify your own binary) b
MyDeveloperDay added a comment.
This has caught me out from time to time, but the presence of such an option
can lead to users mixing clang-format versions which could lead to
flip/flopping of changes, so I'm not 100% sure it won't drive bad behaviour.
However breaking based on the option can b
fodinabor added a comment.
Ping
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86137/new/
https://reviews.llvm.org/D86137
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi
fodinabor created this revision.
fodinabor added reviewers: bkramer, djasper, klimek.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya.
Herald added projects: clang, LLVM.
fodinabor requested review of this revision.
Currently newer clang-format options cannot be included in .clang-f
17 matches
Mail list logo
