[PATCH] D32298: [clang-format] Turn IncompleteFormat into a string

Daniel Jasper via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 20 23:12:11 PDT 2017


djasper added inline comments.


================
Comment at: include/clang/Format/Format.h:1527
+/// non-recoverable syntax error.
 tooling::Replacements reformat(const FormatStyle &Style, StringRef Code,
                                ArrayRef<tooling::Range> Ranges,
----------------
This is a public interface function that possibly we shouldn't just change in case people use clang-format as a library.

Can you leave the old one, with a comment say that it is being deprecated. That one can then just call this new version and set the boolean depending on whether the string is empty.


================
Comment at: lib/Format/UnwrappedLineFormatter.cpp:845
+      if (!Invalid)
+        os << " This might be due to a syntax error at line " << LineNumber
+           << ".";
----------------
I wonder whether this might be confusing when an unwrapped line spans over multiple physical lines. But I also don't have a much better idea on how to phrase this.


================
Comment at: tools/clang-format/ClangFormat.cpp:314
                << FormatChanges.getShiftedCodePosition(CursorPosition)
-               << ", \"IncompleteFormat\": "
-               << (IncompleteFormat ? "true" : "false") << " }\n";
+               << ", \"IncompleteFormat\": \"";
+        outs().write_escaped(IncompleteFormat);
----------------
I think we should not change the JSON here in such a backwards incompatible way. That will break all existing editor integrations that use it. How about adding an additional field "Message:" that gets set when the format is incomplete?


https://reviews.llvm.org/D32298





More information about the cfe-commits mailing list