[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

MyDeveloperDay via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 26 07:52:41 PDT 2021


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



More information about the cfe-commits mailing list