Improving formatting of generics + variable declarations. #240
Conversation
|
|
||
| public System.Text.StringBuilder NamespacedField; | ||
|
|
||
| public static Dictionary<Type, string> PropertiesByType = new Dictionary< |
There was a problem hiding this comment.
I think it makes sense to break on the assignment first, i.e.
public static Dictionary<Type, string> PropertiesByType =
new Dictionary<Type, string>();
In fact, I'm a little confused why this isn't happening already. The assignment expression is the outermost Group() right? It should break on that before the its children (which presumably are the left and right sides)
There was a problem hiding this comment.
There were a few edge cases that popped up due to the changes I was making. I created issues for some of them because this PR was already getting large.
Getting this to break after the = caused a lot of other cases to change. I think this one requires conditional groups (which don't exist yet), or possibly an ifBreak.
#239
| var x = new SomeClass | ||
| { | ||
| Property1 = true | ||
| // should indent |
There was a problem hiding this comment.
Like... who would put a comment there?! 🍌
There was a problem hiding this comment.
Exactly. But I think I ran into this case in one of the repos I test format.
| var task = Task.Factory.StartNew( | ||
| async () => | ||
| { | ||
| var task = Task.Factory.StartNew(async () => { |
There was a problem hiding this comment.
Oaf. Are you sure about this? It's the first time we've opened a curly brace before the line break...
There was a problem hiding this comment.
I opened this to address it #238
But it turned out to be a two minute fix not counting the indentation.
| @@ -54,7 +54,9 @@ public class ClassName | |||
| set; | |||
| } | |||
|
|
|||
| public virtual ICollection<SomeObject> SomeLongNameThatForcesALineBreak { get; set; } = | |||
| public virtual ICollection< | |||
| SomeObject | |||
There was a problem hiding this comment.
We're breaking too eagerly here. public virtual ICollection<SomeObject> fits in one line.
Hypothetically, even if ti didn't, I think the type arguments should be the last group we break.
public virtual
ICollection<SomeLooooooooooooooooooooooooooongType>
SomeLongNameThatForcesALineBreak { get; set; } =
HashSet<SomeLooooooooooooooooooooooooooongType>There was a problem hiding this comment.
This is currently similar to how prettier breaks the following typescript
public SomeName: SomeLongGenericClassThatBreaks<
SomeRandomType,
SomeOtherRandomType
> = new SomeLongGenericClassThatBreaks(someValue, someOtherValue);
Although I'm not sure how much I like how that typescript breaks. I created #243
| @@ -4,6 +4,13 @@ class ClassName | |||
| { | |||
| var query1 = from c in customers select c; | |||
| query1 = from c in customers select c into d select d; | |||
| query1 = | |||
| from c in customers | |||
| join c1 in customers11111111111111111111111111111 | |||
There was a problem hiding this comment.
Looks surprisingly nice 👍
|
These changes caused all kinds of formatting to get real ugly when I was reviewing how the 0.9.5 RC would format the forked public repos I had. I pulled it out of master for now, and will probably spend the next week or two just cleaning up issues related to it. |
This started as just cleaning up the formatting of a generic dictionary in a field but attempting to get that cleaned up broke a number of other test cases without adding all this logic into VariableDeclaration.
This will allow us to handle edge cases better. The previous simple logic handled basic cases well, but didn't handle edge cases cleanly.
I split a couple of things off into separate tickets.
#239 could be better, but this is how prettier currently formats things.
#7 I added another edge case to this which will probably require conditional groups.