I've tried, of course. And my message goes after YAML I/O's, but this way
it's still informative enough for diagnostic purposes. And I thought we can
come back to this once YAML I/O has better support for this.
On May 9, 2013 9:54 AM, "Kim Gräsman" <kim.gras...@gmail.com> wrote:
>
> Looks good!
>
> Did you try provoking a syntax error? Unless it's been short-circuited,
> I think the YAML parsing framework will print a diagnostic before your
> manual llvm::errs() output, but I don't have time to rebuild and check
> right now.
>
>
> ================
> Comment at: lib/Format/Format.cpp:56-64
> @@ -55,1 +55,11 @@
> + } else {
> + StringRef StylesArray[] = { "LLVM", "Google", "Chromium", "Mozilla"
> };
> + ArrayRef<StringRef> Styles(StylesArray);
> + for (size_t i = 0; i < Styles.size(); ++i) {
> + StringRef StyleName(Styles[i]);
> + if (Style == clang::format::getPredefinedStyle(StyleName)) {
> + IO.mapOptional("# BasedOnStyle", StyleName);
> + break;
> + }
> + }
> }
> ----------------
> Maybe move this into clang::format::isPredefinedStyle(const FormatStyle&
> Style)?
>
> That way you don't have to re-define the predefined style names here too.
>
> ================
> Comment at: lib/Format/Format.cpp:61
> @@ +60,3 @@
> + if (Style == clang::format::getPredefinedStyle(StyleName)) {
> + IO.mapOptional("# BasedOnStyle", StyleName);
> + break;
> ----------------
> IIRC, the YAML parser doesn't take "#" comments, only JavaScript-style
> "//", but I'm not 100% sure. Did you try roundtripping a dumped
> configuration?
>
>
> http://llvm-reviews.chandlerc.com/D758
>
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
