conditionals: add support for nested indentation #450
Conversation
|
|
|
Ah, I think I accidentally made the commit on my personal account so the CLA assistant thinks I haven't signed it... edit: yep, all fixed, just signed it on my personal account. |
|
Cool. Thanks @declanvong! I'll try to take a look on the weekend. Very busy next few weeks because of work work. |
|
Bump on this one! I also have a couple more PRs - would you rather I just raise them all immediately? and then we can comment / discuss there? Most of them have linkable issues as well. Otherwise I'm happy to raise them one by one if that makes it less stressful, lol |
There was a problem hiding this comment.
I just realized this PR makes it so that it will have nested indentation AND force a line per expression. There is already an option conditionalExpression.linePerExpression that handles that for conditional expressions and conditional types will maintain their formatting unless "preferSingleLine" is true. I added some failing tests. Would you be able to tackle updating the code to make them pass? If not I can work on it hopefully soon.
Sorry for my delay, as per usual!
|
Ah yep, I can take a look! Note: I've literally just started back at work today and I might be busy with onboarding, so I might take some time as well :) |
|
I checked out the new tests; I think that I will have to add something like Does that sound okay? |
|
Updates:
|
| @@ -2172,9 +2172,14 @@ fn gen_conditional_expr<'a>(node: &'a CondExpr, context: &mut Context<'a>) -> Pr | |||
| let colon_token = context.token_finder.get_first_operator_after(&node.cons, ":").unwrap(); | |||
| let line_per_expression = context.config.conditional_expression_line_per_expression; | |||
| let has_newline_test_cons = node_helpers::get_use_new_lines_for_nodes(&node.test, &node.cons, context.program); | |||
| let has_newline_const_alt = node_helpers::get_use_new_lines_for_nodes(&node.cons, &node.alt, context.program); | |||
There was a problem hiding this comment.
This is just a typo fix, const -> cons
| ? C extends D | ||
| ? c | ||
| : testingTesting | ||
| : testingTestingTestingTesting; |
There was a problem hiding this comment.
@declanvong sorry for my delay here. I just added this file and test, but it fails. I think this is the expected output?
There was a problem hiding this comment.
Yeah, looks like that maybe one of the indent conditions is listening to linePerExpression instead of useNestedIndentation? I'll take a look
There was a problem hiding this comment.
Fixed, but important to note that this change is not backwards compatible, it changes existing formatting, so it'd be a version bump.
There's a test with some existing formatting that I've updated (ConditionalType_All.txt::should format an individual condition that goes over the limit).
Since both linePerExpression and useNestedIndentation are not enabled in that test, it goes from being formatted originally like this:
type Type<T> = T extends string ? number
: T extends hereIsAVeryLongExtendsClause
? testingThis
: boolean;
to this (flattened indentation):
type Type<T> = T extends string ? number
: T extends hereIsAVeryLongExtendsClause
? testingThis
: boolean;
Enabling useNestedIndentation without linePerExpression gives:
type Type<T> = T extends string ? number
: T extends hereIsAVeryLongExtendsClause
? testingThis
: boolean;
which also seems correct.
So there's no way to get the original formatting in this new world, but maybe that's good? if the original formatting was always not technically correct or wanted. after all, that trailing : boolean really is part of the inner conditional, not the outer one, so I feel like the new formatting is better anyway as it makes that more obvious.
|
Hopefully this PR can be resurrected, the lack of this feature is one of my only gripes with dprint! |
|
@declanvong @dsherret |
|
Hey, @dsherret can we do something to help move this along? We would love to see it merged. |
|
Due to the inactivity of this repository, what about fork this dprint plugin and maintain by ourselves? |
We've had this in our fork for over a year now, and I noticed #432 pop up.
This PR adds support for nested indentation in conditional types and expressions, so that it formats as a tree.