This patch replaces the boolean IncompleteFormat that is used to notify the client if an unrecoverable syntax error occurred by a struct that also contains a line number.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
include/clang/Format/Format.h | ||
---|---|---|
1527 ↗ | (On Diff #95975) | 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. |
lib/Format/UnwrappedLineFormatter.cpp | ||
845 ↗ | (On Diff #95975) | 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. |
tools/clang-format/ClangFormat.cpp | ||
314 ↗ | (On Diff #95975) | 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? |
- Removed double declaration
lib/Format/UnwrappedLineFormatter.cpp | ||
---|---|---|
845 ↗ | (On Diff #95975) | We may wait and see how much people will be confused. |
include/clang/Format/Format.h | ||
---|---|---|
1519 ↗ | (On Diff #96120) | Maybe we should invert this and make this FormatComplete or something? Otherwise this is bound to lead to an unnecessary double-negative. |
1522 ↗ | (On Diff #96120) | s/when/at which/ |
1523 ↗ | (On Diff #96120) | no comma, I think. |
1524 ↗ | (On Diff #96120) | Hm. Something we might need to be thinking about here is whether this should be the line before or after formatting. So specifically, if I format: int a = 1; f( Then, this gets reformatted to: int a = 1; f( Would we want the Line to be 2 or 3? |
lib/Format/Format.cpp | ||
1886 ↗ | (On Diff #96120) | nit: There shouldn't be a line break here. |
1889 ↗ | (On Diff #96120) | In LLVM style, there should be a line break here. |
include/clang/Format/Format.h | ||
---|---|---|
1524 ↗ | (On Diff #96120) | I'd go with the original line, because I think it might be more useful and because it's easier to implement. |
include/clang/Format/Format.h | ||
---|---|---|
1524 ↗ | (On Diff #96120) | Even better: the line after formatting is functionally dependent on the line before formatting and the replacements. I see two use-cases: clients that refuse to format in case of syntax error, then they can just use the original line, and clients that apply the partial replacements, which can compute the line after formatting from them and the original line. |