- User Since
- Oct 11 2016, 5:32 AM (182 w, 11 h)
Looks good! The linter suggests a few formatting fixes.
A sample of snippets that this misformats:
I think situations where we infer M(x) to expand to a "type" without const/volatile close-by are too ambiguous an we will have the opposite problem: formatting certain binary expressions as type pointer/references. Some sort of a list of macros that clang-format assumes expand to a "type" could eliminate ambiguities, but has its own problems.
I think the case where there's a const/volatile/typename/typedef etc. around M(x) is very interesting; not sure if there are many practical ambiguities there.
There was an idea of giving a bunch of #define-s to clang-format (maybe with a small subset of what's possible) and use these rules to decide how to format matching forms, but that's hard (requires some sort of "virtual expanded token sequences" that the formatter should undestrand how to handle) to do and I don't know what's the status of that.
Fri, Apr 3
Thu, Apr 2
Tue, Mar 31
Mon, Mar 30
Thu, Mar 26
I think this is an improvement. Accepting with a nit. Thank you!
@MyDeveloperDay : sorry, I must have not refreshed this page and didn't realize you already gave an LGTM until I posted the comment.
Mon, Mar 23
This is OK with the comment in the code. Thank you!
Awesome! Thank you!
Thu, Mar 19
Wed, Mar 11
Mon, Mar 9
The new pre-merge checks are cool! Please clang-format the updated lines accordingly.
Looks good! I'll stamp this after https://reviews.llvm.org/D75731 gets merged and this rebased on top of it.
Mar 6 2020
sorry could you please take another look at the comments I left
Mar 5 2020
Merging commits into 1 patch
Remove redundant check.
Mar 4 2020
Looks good with 1 suggestion.
Mar 3 2020
Here's some empirical ideas about the approach: In clang-format, braces can be handled in two quite distinct ways, controlled by BraceBlockKind: BK_Block and BK_BracedInit.
BK_Block is for braces that open blocks that usually are at one level deeper and consist of a sequence of statements and other constructs.
BK_BracedInit is for initializer lists, dictionaries and other similar syntactics that are somewhat more appropriate to put together into a line.
The level of granularity of detailed formatting in clang-format is an unwrapped line, which is conceptually a sequence of tokens that "make sense together as a single line" (without going much into style details and ignoring column limits). Separate statements are for example put on separate unwrapped lines. The formatting information flowing between unwrapped lines includes just higher-level data such as the current nesting depth.
The brace detection is handled primarily in UnwrappedLineParser::calculateBraceTypes, which happens quite early during the parsing of the input sequence.
Thank you for the explanations! Looking good!
That's a bit of a hack (we probably need to hook up full-fledged argument list parsing for complete handling), but this seems like a good trade-off between heuristics and results right now.
Mar 2 2020
Sorry, did the last round of comments not make sense?
Feb 28 2020
Feb 27 2020
This is very nice! Brings the C# handling of the colon closer to other languages.
Feb 26 2020
Feb 25 2020
As a general comment, we should consider which of these special cases can already be simulated by a combination of existing style options:
There are several Spaces* options there that can be used to independently control various spacing around parenthesis.
This works nice for many examples, however I want to point out some limitations of using WhitespaceManager for this.
The alignment code in WhitespaceManager is conceptually at the end of the formatting pipeline, where all of the formatting decisions (where to break a line and what indentation to use for the following line) have already been fixed.
The code here attempts to do a post-processing pass that re-arranges some tokens, without being able to "deeply" affect / fix formatting.
As such is usually used either for non-compound constructs (aligning trailing comments) or in cases where the alignment is best-effort.
The tricky part of this is that, if not careful, aligning using the WhitespaceManager could introduce instances of lines that start going over the column limit as a result of alignment.
If you attempt to re-format the resulting code then, clang-format might take different formatting decisions for such lines, resulting in unstable formatting.
This is pretty cool!
Feb 20 2020
Feb 11 2020
Feb 5 2020
Thanks! Looks like this is OK also for Java, I'm not sure about ObjC. If it happens to affect other languages negatively, we can restrict by adding language-specific checks.
This seems OK, but it feels a bit messy that the text [target: becomes the opening square bracket. I think this might be just good enough for these attributes, so I'm ok with this for now. But a more principled solution (for later, if we hit any dead ends with this) would be to introduce a new AnnotateToken type, like TT_CSharpAttributeTarget and make sure these are annotated properly.
Jan 31 2020
Jan 30 2020
Jan 29 2020
This is pretty cool! Just a couple of nits.
Jan 28 2020
Jan 27 2020
An alternative approach (I'm not endorsing this) that would *in theory* circumvent non-idempotency issue (but would be more fiddly~fiddly to implement and spill across different layers of the formatter):
This is just a rough idea and can have multiple issues in itself. In general the trick is that if we can teach the token penalty computer and the format outputter about this, we should be able to use it together with other formatting decisions.
- add a new synthetic (annotated) token type, something like TT_PossiblyMissingTrailingComma, that would capture the length-0 text range where this could be inserted;
- use the algorithm here to splay it into the annotated token stream;
- mark it so that no newline can be inserted before such a token
- modify the penalty calculation algorithm to penalize for inserting a newline after this token: if we're inserting a newline after this token and if this token was inserted at a position at or past the maximal number of columns, add a PenaltyExcessCharacter for it.
- during outputting, if this token is followed by a newline, output it as ,; otherwise output it as an empty string or do not output it.
Jan 24 2020
Should I commit this for you?
FYI I've started a conversation on whether SLS_All should be the default Google style. But the option I think is useful in either case.
Jan 23 2020
I'll submit this for you.
I looked up the C# reference and all the examples are top-level, so having top-level test cases here would be useful, as people would expect copy-pasting an example snippet to just work.
Sorry about asking you to go back-and-forth.
Jan 22 2020
Jan 20 2020
Jan 17 2020
- Fix typo
- Fix typo
- Address review comments