[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