MyDeveloperDay requested changes to this revision. MyDeveloperDay added a comment. This revision now requires changes to proceed.
Thank you for your submission, but... 1. This is not the way to change this file, its autogenerated from clang/include/Format/Format.h using clang/docs/tools/dump_format_style.py 2. Why do you think it should be std::string? To be honest this file pretty much describes the `YAML` file format of `.clang-format` so actually I would suggest it saying string was more correct I think the main problem is options like this: F18679590: image.png <https://reviews.llvm.org/F18679590> Here we say its a `std::vector<RawStringFormat>` which doesn't really tell you much, as its actually just a `YAML` array of `RawStringFormat` records without actually telling you what a `RawStringFormat` record contains RawStringFormats: - Language: TextProto Delimiters: - 'pb' - 'proto' EnclosingFunctions: - 'PARSE_TEXT_PROTO' BasedOnStyle: google - Language: Cpp Delimiters: - 'cc' - 'cpp' BasedOnStyle: llvm CanonicalDelimiter: 'cc' So whilst I see that you were being consistent I kind of feel its in the wrong direction, we should be moving away from using `C++` names here but more explaining what the `YAML` should look like. Alas the C++ in Format.h is important because the types are needed because we are generating the documentation directly from the code. But in most of places we are using an enumeration like this one. F18679643: image.png <https://reviews.llvm.org/F18679643> we are not saying `clang::FormatStyle::RefernceAlignmentStyle` and I'm not convinced we'd want to I hope that helps, but thank you for the patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108765/new/ https://reviews.llvm.org/D108765 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
