[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 09:53:46 PDT 2021


MyDeveloperDay added subscribers: HazardyKnusperkeks, owenpan, krasimir, sammccall, curdeius, klimek.
MyDeveloperDay added a comment.

In D108765#2967363 <https://reviews.llvm.org/D108765#2967363>, @FederAndInk wrote:

> Thank you for your explanations, I understand now.
>
> But as I look into `clang/docs/tools/dump_format_style.py` I see that it does not entirely generate `clang/docs/ClangFormatStyleOptions.rst` it replaces the lines between `{START,END}_FORMAT_STYLE_OPTIONS`
>
> I understand your point, but as of now, the inconsistency comes from the part that is not auto-generated, are you suggesting editing `dump_format_style.py` to have simpler types such as `string`? Then how should we replace `std::vector`? Something like `Type[]` e.g. `string[]`?
>
> Or maybe we should first include `BasedOnStyle` into `dump_format_style.py`. Then take care of how to render types?
>
> What do you suggest? I am genuinely asking, as I really don't know what would be the best way to do things. Maybe we should include other people? I don't really know who to add as reviewers for that, but I think, the way to show types, should be discussed?
>
> As for detailing `RawStringFormat`, it wasn't the purpose of this patch, and maybe it should have its own?

You are correct the file isn't 100% generated and some of it comes from another .h file too.

But now we have you interesting in making a contribution which you clearly are lets think about how we might do this.

To hook into the clang-format team I always recommend adding the #clang-format <https://reviews.llvm.org/tag/clang-format/> project, (which I added to this review), but also I recommend passing the review via @krasimir , @HazardyKnusperkeks , @curdeius there are some others who are hear often like @owenpan and @sammccall and of course @klimek (who started all this). Please also of course add me @MyDeveloperDay I try to check the reviews daily as one of my frustrations was not being able to get things reviewed so I try to be pretty active.

>From my perspective I do like the idea of substituting out the `std::string` and `std::vector` for something like `string[]` how about we start with something simple like trying to fix the cases for `AttributeMacros (std::vector<std::string>)` maybe with just simple substitution.

We can pass that via the rest of the team and see what they feel even if we ultimately have both

AttributeMacros (in configuration `string[]`)

or something like that, I'm not con convinced anyone is using this documentation to know its a std::vector!


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