Page MenuHomePhabricator

djasper (Daniel Jasper)
User

Projects

User does not belong to any projects.

User Details

User Since
Jul 10 2012, 10:15 AM (375 w, 2 d)

Recent Activity

May 12 2019

djasper accepted D61663: [clang-format] Fix a JavaScript import order bug..

Generally, upload patches with the full file as context (that will prevent Phabricator's "Context not available")
But this change looks good. Thank you.

May 12 2019, 10:46 PM · Restricted Project

Apr 18 2019

djasper accepted D60558: [clang-format] Fix indent of trailing raw string param after newline.

Looks good. Thank you.

Apr 18 2019, 7:36 AM · Restricted Project

Feb 1 2019

djasper accepted D57604: [clang-format] Fix breaking of qualified operator.

Thank you.

Feb 1 2019, 2:17 PM · Restricted Project

Jan 11 2019

djasper added a comment to D53072: [clang-format] Add a new extra command line option for ide-specific formatting.

Ok, but this behavior is still intended. You are setting clang-format to a format where it is breaking after binary operators and then added a break before a binary operator. clang-format assumes that this is not intended and that you will want this cleaned up.

Jan 11 2019, 8:14 AM

Jan 9 2019

djasper added a comment to D55964: [clang-format][TableGen] Don't add spaces around items in square braces..

Look at getGoogleStyle(). It has a bunch of language-specific configs at the bottom. You can do the same for TableGen in getLLVMStyle().

Jan 9 2019, 10:37 AM · Restricted Project
djasper added a comment to D55964: [clang-format][TableGen] Don't add spaces around items in square braces..

This seem to conceptually be a list of things rather than an array subscript, though, right? Could we alternatively set SpacesInContainerLiterals to false for LK_TableGen?

Jan 9 2019, 9:18 AM · Restricted Project

Jan 7 2019

djasper added a comment to D53072: [clang-format] Add a new extra command line option for ide-specific formatting.

Without understanding this in more detail I do not know how to move forward with this. What this patch is describing is what should already be the case with ColumnLimit set to zero. If it isn't this might be a bug or there might be a different way to move forward. However, the flag as is does not make sense to me without more information.

Jan 7 2019, 4:46 AM
djasper added a comment to D53072: [clang-format] Add a new extra command line option for ide-specific formatting.
$ cat /tmp/test.cc
int foo(int a,
           int b) {
      f();
  }
Jan 7 2019, 4:38 AM

Dec 29 2018

djasper added a comment to D53072: [clang-format] Add a new extra command line option for ide-specific formatting.

I don't quite understand. What you are describing should already be the behavior with ColumnLimit=0 and I think your test should pass without the new option. Doesn't it?

Dec 29 2018, 1:00 PM

Nov 22 2018

djasper accepted D54795: [clang-format] Do not treat asm clobber [ as ObjCExpr, refined.
Nov 22 2018, 6:28 AM

Nov 21 2018

djasper added a comment to D54795: [clang-format] Do not treat asm clobber [ as ObjCExpr, refined.

Does this also work for _asm and __asm?

Nov 21 2018, 8:27 AM

Oct 30 2018

djasper added a comment to D52676: [clang-format] tweaked another case of lambda formatting.

I think this roughly looks fine. Krasimir, any thoughts?

Oct 30 2018, 7:28 AM · Restricted Project

Oct 23 2018

djasper added inline comments to D52676: [clang-format] tweaked another case of lambda formatting.
Oct 23 2018, 7:53 AM · Restricted Project

Oct 18 2018

djasper added a comment to D52676: [clang-format] tweaked another case of lambda formatting.

Ok, I think I agree with both of you to a certain extent, but I also think this change as is, is not yet right.

First, it does too much. The original very first example in this CL is actually not produced by clang-format (or if it is, I don't know with which flag combination). It is a case where the lambda is the last parameter.

Right, I cheated and created that example by hand. My apologies for the confusion. I've just pasted real code that I pumped through clang-format. Please take a look at the updated summary.

Second, I agree that the original behavior is inconsistent in a way that we have a special cases for when the lambda is the very first parameter, but somehow seem be forgetting about that when it's not the first parameter. I'd be ok with "fixing" that (it's not a clear-cut bug, but I think the alternative behavior would be cleaner). However, I don't think your patch is doing enough there. I think this should be irrespective of bin-packing (it's related but not quite the same thing),

Also there is a special case for multiple lambdas. It forces line breaks. That aside, for the single-lambda case, are you suggesting that it is always "pulled up", irrespective of its place? That would contradict the direction I am trying to take as I like BinPackArguments: false and so long lamba args go to their own lines. This looks very much in line with what bin packing is, but not exactly the same. Obviously, I can add a new option favor line breaks around multi-line lambda.

Oct 18 2018, 7:04 PM · Restricted Project
djasper added a comment to D52676: [clang-format] tweaked another case of lambda formatting.

Ok, I think I agree with both of you to a certain extent, but I also think this change as is, is not yet right.

Oct 18 2018, 2:38 PM · Restricted Project

Aug 15 2018

djasper committed rC339803: clang-format: Change Google style wrt. the formatting of empty messages..
clang-format: Change Google style wrt. the formatting of empty messages.
Aug 15 2018, 12:08 PM
djasper committed rL339803: clang-format: Change Google style wrt. the formatting of empty messages..
clang-format: Change Google style wrt. the formatting of empty messages.
Aug 15 2018, 12:08 PM

Aug 1 2018

djasper accepted D50132: [clang-format] Add some text proto functions to Google style.

Looks good.

Aug 1 2018, 5:32 AM
djasper added a comment to D50078: clang-format: support aligned nested conditionals formatting.

I don't have very strong opinions here and I agree that we will need to improve the conditional formatting, especially as this kind of ternary-chaining is becoming more popular because of constexpr.

Aug 1 2018, 2:15 AM · Restricted Project

Jul 30 2018

djasper added a comment to D49580: [clang-format] Adding style option for absolute formatting.

Ok, so IIUC, understanding that @end effective ends a section much like "}" would address the currently observed problems?

Jul 30 2018, 3:01 AM

Jul 23 2018

djasper added a comment to D49580: [clang-format] Adding style option for absolute formatting.

In my opinion, this only addresses one edge case where clang-format -lines output is not identical with a full reformatting. I believe they cannot all usefully be avoided. As such, I am unsure that this option carries its weight of making the implementation more complex.
How often does this happen for you? Why can't you just format the additional incorrect line?

Jul 23 2018, 10:08 AM
djasper added a comment to D49580: [clang-format] Adding style option for absolute formatting.

Could you explain what problem this is fixing?

Jul 23 2018, 3:19 AM

Jul 12 2018

djasper added inline comments to D40988: Clang-format: add finer-grained options for putting all arguments on one line.
Jul 12 2018, 3:36 AM · Restricted Project
djasper added inline comments to D40988: Clang-format: add finer-grained options for putting all arguments on one line.
Jul 12 2018, 3:35 AM · Restricted Project

Jun 29 2018

djasper accepted D48760: [clang-format] Support additional common functions for text proto formatting.
Jun 29 2018, 7:15 AM

Jun 26 2018

djasper accepted D48363: [clang-format] Enable text proto formatting in common functions.

Looks good.

Jun 26 2018, 2:03 AM

Jun 12 2018

djasper edited reviewers for D48098: clang-format-diff: Make it work with python3 too, added: krasimir; removed: djasper.
Jun 12 2018, 11:26 PM
djasper added a comment to D42787: clang-format: do not add extra indent when wrapping last parameter.

You are right that this behavior is what the code authors, but also many other people, like to have and so it is what is engrained in clang-format. There are likely about a million things that fall into the same category. Now we might find that the current default is actually wrong and we have changed many defaults in the past. I don't believe that's the case here and there are more opinions on this thread.

Jun 12 2018, 9:44 AM

Jun 11 2018

djasper accepted D43015: clang-format: Introduce BreakInheritanceList option.

Looks good. Sorry for the delay.

Jun 11 2018, 2:21 AM
djasper added a comment to D42787: clang-format: do not add extra indent when wrapping last parameter.

The normal rule for formatting options apply. If you can dig up a public style guide and a project of reasonable size where it is used, we can add an option.

Jun 11 2018, 2:18 AM

Jun 5 2018

djasper edited reviewers for D47759: [Format] Do not use a global static value for EOF within ScopedMacroState., added: klimek; removed: djasper.
Jun 5 2018, 4:25 AM · Restricted Project

May 20 2018

djasper added inline comments to D40988: Clang-format: add finer-grained options for putting all arguments on one line.
May 20 2018, 11:41 PM · Restricted Project
djasper edited reviewers for D47095: [clang-format/ObjC] Correctly parse Objective-C methods with 'class' in name, added: klimek; removed: djasper.
May 20 2018, 11:21 PM

May 8 2018

djasper accepted D46519: [clang-format] Respect BreakBeforeClosingBrace while calculating length.

Generally looks good.

May 8 2018, 3:43 AM
djasper added inline comments to D46519: [clang-format] Respect BreakBeforeClosingBrace while calculating length.
May 8 2018, 2:20 AM

Apr 27 2018

djasper accepted D46143: [clang-format/ObjC] Use getIdentifierInfo() instead of tok::identifier.

Looks good, thank you.

Apr 27 2018, 10:17 AM

Apr 20 2018

djasper added inline comments to D40988: Clang-format: add finer-grained options for putting all arguments on one line.
Apr 20 2018, 12:47 AM · Restricted Project

Apr 19 2018

djasper accepted D45373: [clang-format] Don't remove empty lines before namespace endings.

Looks good.

Apr 19 2018, 5:14 AM

Apr 12 2018

djasper accepted D45526: [clang-format] Do not break after ObjC category open paren.

Looks good.

Apr 12 2018, 7:58 AM
djasper added a comment to D45526: [clang-format] Do not break after ObjC category open paren.

I understand that, but the test example does not break after the closing paren. It breaks after the subsequent "<".

Apr 12 2018, 6:58 AM
djasper added a comment to D45526: [clang-format] Do not break after ObjC category open paren.

Both patch and comment inside patch say "break after closing paren" and yet that's not what the test or example in the description actually do. Why is that?

Apr 12 2018, 5:36 AM
djasper accepted D45004: [clang-format] Always indent wrapped Objective-C selector names.
Apr 12 2018, 5:33 AM
djasper accepted D45521: [clang-format] Improve ObjC guessing heuristic by supporting all @keywords.

Looks good.

Apr 12 2018, 5:31 AM
djasper accepted D45498: [clang-format] Don't insert space between ObjC class and lightweight generic.

Looks good.

Apr 12 2018, 5:30 AM

Apr 6 2018

djasper added a comment to D45004: [clang-format] Always indent wrapped Objective-C selector names.

Ok, you know the ObjC community much better than I do. If you think that adding the indentation for ObjC irrespective of the option works for most people, I am happy to go with it.

Apr 6 2018, 4:45 AM

Apr 5 2018

djasper added a comment to D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style).

Please read:

Apr 5 2018, 11:35 PM · Restricted Project
djasper added a comment to D45004: [clang-format] Always indent wrapped Objective-C selector names.

I'd go to great lengths to avoid adding new config options and so I don't think this would be a bad trade-off to make.

Apr 5 2018, 9:54 AM
djasper accepted D45185: [clang-format] Support lightweight Objective-C generics.

Looks good, thank you!

Apr 5 2018, 8:15 AM
djasper accepted D44996: [clang-format] Ensure ObjC selectors with 0 args are annotated correctly.
Apr 5 2018, 2:37 AM
djasper added a comment to D45004: [clang-format] Always indent wrapped Objective-C selector names.

I still really believe that these config options do no make sense and are actively confusing.

Apr 5 2018, 1:55 AM

Apr 3 2018

djasper added inline comments to D45185: [clang-format] Support lightweight Objective-C generics.
Apr 3 2018, 11:44 PM

Apr 2 2018

djasper accepted D45169: [clang-format/ObjC] Do not detect "[]" as ObjC method expression.

Looks good. I like option 2 :).

Apr 2 2018, 10:25 PM
djasper accepted D45168: [clang-format/ObjC] Do not insert space after opening brace of ObjC dict literal.

Looks good.

Apr 2 2018, 10:25 PM

Mar 30 2018

djasper accepted D44994: [clang-format] Ensure wrapped ObjC selectors with 1 arg obey IndentWrappedFunctionNames.

Looks good.

Mar 30 2018, 8:03 AM
djasper added inline comments to D44996: [clang-format] Ensure ObjC selectors with 0 args are annotated correctly.
Mar 30 2018, 8:02 AM

Mar 29 2018

djasper added inline comments to D44994: [clang-format] Ensure wrapped ObjC selectors with 1 arg obey IndentWrappedFunctionNames.
Mar 29 2018, 8:54 AM
djasper added a comment to D45004: [clang-format] Always indent wrapped Objective-C selector names.

Well, I disagree. It says: "If you break after the return type of a function declaration or definition, do not indent."

Mar 29 2018, 8:44 AM
djasper added inline comments to D44994: [clang-format] Ensure wrapped ObjC selectors with 1 arg obey IndentWrappedFunctionNames.
Mar 29 2018, 5:27 AM
djasper added a reviewer for D45004: [clang-format] Always indent wrapped Objective-C selector names: thakis.
Mar 29 2018, 5:14 AM
djasper added a comment to D45004: [clang-format] Always indent wrapped Objective-C selector names.

Do we have a public style guide that explicitly says to do this?
That's a requirement for new style options as per https://clang.llvm.org/docs/ClangFormatStyleOptions.html#adding-additional-style-options.

Mar 29 2018, 5:14 AM

Mar 27 2018

djasper accepted D44816: [clang-format] Do not insert space before closing brace in ObjC dict literal.

Yeah, it's one of these things where neither way would be totally intuitive to everyone.

Mar 27 2018, 7:55 AM
djasper accepted D44831: [clang-format] Refine ObjC guesser to handle child lines of child lines.

Looks good.

Mar 27 2018, 7:54 AM
djasper accepted D44816: [clang-format] Do not insert space before closing brace in ObjC dict literal.

Generally looks good, one minor simplification.

Mar 27 2018, 5:27 AM
djasper added inline comments to D44831: [clang-format] Refine ObjC guesser to handle child lines of child lines.
Mar 27 2018, 2:09 AM

Mar 26 2018

djasper added inline comments to D44831: [clang-format] Refine ObjC guesser to handle child lines of child lines.
Mar 26 2018, 1:33 AM

Mar 23 2018

djasper added inline comments to D44790: [clang-format] Fix ObjC style guesser to also iterate over child lines.
Mar 23 2018, 5:48 AM

Mar 22 2018

djasper accepted D44790: [clang-format] Fix ObjC style guesser to also iterate over child lines.

Looks good, thank you!

Mar 22 2018, 10:29 AM
djasper accepted D42684: clang-format: Allow optimizer to break template declaration..

Some last comments, but basically looks good.

Mar 22 2018, 9:33 AM
djasper accepted D43290: clang-format: tweak formatting of variable initialization blocks.

Generally looks good.

Mar 22 2018, 8:04 AM
djasper committed rL328201: clang-format: Narrow down raw string literal line break exception..
clang-format: Narrow down raw string literal line break exception.
Mar 22 2018, 7:46 AM
djasper committed rC328201: clang-format: Narrow down raw string literal line break exception..
clang-format: Narrow down raw string literal line break exception.
Mar 22 2018, 7:46 AM
djasper committed rL328200: clang-format: Fix SpacesInParentheses with fully qualified names..
clang-format: Fix SpacesInParentheses with fully qualified names.
Mar 22 2018, 7:33 AM
djasper committed rC328200: clang-format: Fix SpacesInParentheses with fully qualified names..
clang-format: Fix SpacesInParentheses with fully qualified names.
Mar 22 2018, 7:33 AM

Mar 21 2018

djasper accepted D44632: [clang-format] Add a few more Core Graphics identifiers to ObjC heuristic.

Looks good.

Mar 21 2018, 8:06 AM
djasper accepted D44692: [clang-format] Don't insert space between r_paren and 'new' in ObjC decl.

Looks good.

Mar 21 2018, 8:05 AM
djasper accepted D43957: Fixing issue where a space was not added before a global namespace variable when SpacesInParentheses is set.

Please generally upload diffs with the full file as context so that Phabricator can properly expand the context where necessary.

Mar 21 2018, 5:27 AM

Mar 20 2018

djasper added inline comments to D44632: [clang-format] Add a few more Core Graphics identifiers to ObjC heuristic.
Mar 20 2018, 8:37 AM
djasper added inline comments to D44632: [clang-format] Add a few more Core Graphics identifiers to ObjC heuristic.
Mar 20 2018, 2:40 AM
djasper added a comment to D44634: [clang-format] Detect Objective-C for #import <Foundation/Foundation.h>.

I'd really like to not parse include/import statements this way. Can you send me examples of headers where we fail to recognize them as ObjC and this matters (happy for you to send me examples offline).

Mar 20 2018, 2:35 AM

Mar 12 2018

djasper accepted D43902: [clang-format] Don't detect C++11 attribute specifiers as ObjC.

Ok, looks good.

Mar 12 2018, 8:34 AM
djasper committed rL327255: clang-format: Properly handle implicit string concatenation in text protos.
clang-format: Properly handle implicit string concatenation in text protos
Mar 12 2018, 3:35 AM
djasper committed rC327255: clang-format: Properly handle implicit string concatenation in text protos.
clang-format: Properly handle implicit string concatenation in text protos
Mar 12 2018, 3:35 AM
djasper committed rC327253: Don't re-format raw string literal contents when formatting is disable.
Don't re-format raw string literal contents when formatting is disable
Mar 12 2018, 3:15 AM
djasper committed rL327253: Don't re-format raw string literal contents when formatting is disable.
Don't re-format raw string literal contents when formatting is disable
Mar 12 2018, 3:14 AM

Mar 8 2018

djasper accepted D43906: [clang-format] Improve detection of Objective-C block types.

Nice.. Removed a lot of complexity :). Let's see whether this heuristic is good enough.

Mar 8 2018, 4:01 PM
djasper added inline comments to D43902: [clang-format] Don't detect C++11 attribute specifiers as ObjC.
Mar 8 2018, 3:56 PM

Mar 7 2018

djasper added inline comments to D43906: [clang-format] Improve detection of Objective-C block types.
Mar 7 2018, 4:48 AM

Mar 6 2018

djasper added a comment to D43906: [clang-format] Improve detection of Objective-C block types.

Right. So the difference is that for blocks, there is always a "(" after the "(^.....)". That should be easy to parse, no?

Mar 6 2018, 1:32 PM
djasper added inline comments to D43906: [clang-format] Improve detection of Objective-C block types.
Mar 6 2018, 11:34 AM
djasper added inline comments to D43906: [clang-format] Improve detection of Objective-C block types.
Mar 6 2018, 5:37 AM

Mar 5 2018

djasper added a comment to D43906: [clang-format] Improve detection of Objective-C block types.

Would it be enough to only add the block type case? With the macro false positive, there won't be an open paren after the closing paren, right?

Mar 5 2018, 10:54 AM
djasper added inline comments to D43904: [clang-format] Improve detection of ObjC for-in statements.
Mar 5 2018, 10:45 AM
djasper added inline comments to D43902: [clang-format] Don't detect C++11 attribute specifiers as ObjC.
Mar 5 2018, 10:40 AM

Mar 2 2018

djasper added a comment to D42787: clang-format: do not add extra indent when wrapping last parameter.

We have talked about that and none of us agree.

Mar 2 2018, 7:09 AM
djasper added a comment to D42787: clang-format: do not add extra indent when wrapping last parameter.

If people don't care about this case, we might as well merge this :-) Just kidding.

The tweak matches both our expectation, the auto-indent behaviour of IDE (not to be trusted, but still probably of 'default' behaviour for many people, esp. when you don't yet use a formatter), and its seems our code base is generally formatted like this. I think it is quite more frequent than 50 times in whole LLVM/Clang, but I have no actual numbers; do you have a magic tool to search for such specific "code pattern", or just run clang-format with one option then the next and count the differences ?

Mar 2 2018, 6:48 AM
djasper added a comment to D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch.

Got two responses so far.

Mar 2 2018, 6:41 AM

Mar 1 2018

djasper added a comment to D42787: clang-format: do not add extra indent when wrapping last parameter.

Are you sure that you are even addressing an important case? I have done some research on our codebase and this is something that happens incredibly rarely. The reason is that you have to have a very specific combination of line length, where the last parameter does not fit on one line if indented back to align with the open paren while it does fit on multiple lines if indented right of the comma. In all of LLVM/Clang, there seem to be only about 50 cases. How certain are you that people actually care?

Mar 1 2018, 7:12 AM
djasper added a comment to D42684: clang-format: Allow optimizer to break template declaration..

Indeed, seems to apply to classes as well. Maybe I was mislead by my testing, where I did not get the case (possibly because we use ConstructorInitializerAllOnOneLineOrOnePerLine=true, so the continuation indenter only sees "short" class declarations unless breaking the template is required because the name is too long).

Mar 1 2018, 7:03 AM

Feb 28 2018

djasper accepted D32525: [clang-format] Add SpaceBeforeColon option.

Ah, I thought it was somehow possible to create:

Feb 28 2018, 10:00 AM
djasper added inline comments to D32525: [clang-format] Add SpaceBeforeColon option.
Feb 28 2018, 8:36 AM