MyDeveloperDay marked 4 inline comments as done.
MyDeveloperDay added inline comments.
================
Comment at: clang/docs/ClangFormatStyleOptions.rst:2343
Use C++14-compatible syntax.
+ ``Cpp11``: deprecated alias for ``Latest``
----------------
sammccall wrote:
> sammccall wrote:
> > I'm not sure why this is grouped here. Did you intend this to be under
> > `LS_Latest`?
> `Cpp11` is a deprecated alias for `Latest`, for historical reasons.
yes correct I'll make that change with regard to its position.
================
Comment at: clang/docs/tools/dump_format_style.py:175
+ val = line.replace(',', '')
+ pos = val.find(" // ")
+ if (pos != -1):
----------------
sammccall wrote:
> MyDeveloperDay wrote:
> > MyDeveloperDay wrote:
> > > mitchell-stellar wrote:
> > > > MyDeveloperDay wrote:
> > > > > mitchell-stellar wrote:
> > > > > > This seems quite flimsy to me, as it depends on an undocumented
> > > > > > comment style. It is true that if the file(s) in question are
> > > > > > properly clang-formatted, then this would probably not fail, but it
> > > > > > does not appear to be a very robust solution.
> > > > > I'd tend to agree, but this whole dump_format_style.py is flimsy..
> > > > > take a look at this review {D31574}
> > > > >
> > > > > When you added this line, you forgot the third /
> > > > >
> > > > > ```// Different ways to wrap braces after control statements.```
> > > > >
> > > > > Also, the extra empty line in the LanguageStandard both caused the
> > > > > whole python file to fail with an exception.
> > > > >
> > > > > Do you have a suggestion for something better? (which doesn't leave
> > > > > the Format.h looking too odd)
> > > > I would go back to the `/// c++03: Parse and format as C++03.` style.
> > > > `///` is a Doxygen comment, and I think documentation should be
> > > > generated solely from Doxygen comments, even if it requires a bit of
> > > > post-processing. (The extra `/` needed after `//` in the ticket you
> > > > mentioned is justified.)
> > > The Doxygen documentation is used for source-level documentation, this is
> > > user-level documentation which the restructured text output .rst is used.
> > >
> > > In the past the ClangFormatStyleOpions.rst has been generated from the
> > > Format.h via this script, we should break that.
> > >
> > > The "In configuation" part is super important because it explains to user
> > > what to put into their .clang-format file.
> > >
> > > We have to either have some form of markup that says `LS_Cpp03 == c++03`
> > > in the documentation
> > *we shouldn't break that*
> > The "In configuation" part is super important because it explains to user
> > what to put into their .clang-format file.
>
> Honestly, I'm not sure why the docs say "LS_Foo (in configuration: Foo)"
> rather than just "Foo" - why do users care what the enum is?
>
> But this is an existing practice, and should be changed separately if at all.
I have a tendency to agree with you here.. who cares about the LK_ in the
LK_Cpp value?
{F10569311}
However I know as a clang-format developer I really care about seeing them from
the perspective of being able to easily search in the code for things like
`BCIS_BeforeComma`, otherwise, it's harder for me to work out which setting
goes with which setting without going into Format.h and searching (but that's
just me being lazy), but from a users perspective I wonder how many people put
the enum name in the configuration by mistake..
Oh dear... it turns out this is a problem
https://github.com/search?l=YAML&q=LK_Cpp&type=Code
From time it time it appears people are using the enum name incorrectly.
{F10569361}
@klimek maybe we should consider making this to make it a little clearer.
{F10569372}
I feel we might be guiding people incorrectly.
{F10569374}
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69433/new/
https://reviews.llvm.org/D69433
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
