-
Notifications
You must be signed in to change notification settings - Fork 122
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add formatting for extension type
declarations.
#1276
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of changes then LGTM.
@Anno<int, int>() | ||
extension type const A<S, T>.name( | ||
@required Map<int, int> a) | ||
implements I<S>, J<T> {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't look great to me. There's an implicit principle in the formatter that adjacent lines shouldn't have the same indentation but for different reasons. In this case, line three is implemented +4 because of the representation type's parameter list and the next line is indented +4 because it's a split implements
clause.
If the representation type splits, I would indent it past the implements
clause, like:
@Anno<int, int>()
extension type const A<S, T>.name(
@required Map<int, int> a)
implements I<S>, J<T> {}
(Eventually, with the new style, it will look more like, maybe:
@Anno<int, int>()
extension type const A<S, T>.name(
@required Map<int, int> a,
) implements I<S>, J<T> {}
)
To implement this, try moving:
builder.startRule(CombinatorRule());
Above:
visit(node.typeParameters);
visit(node.representation);
And then you might need to wrap another nestExpression()
unnest()
pair around:
builder.startRule(CombinatorRule());
visit(node.implementsClause);
builder.endRule();
(I could be wrong about this. There tends to be some trial and error with hacking on the formatter. That's one of the reasons I want to move to a better internal representation.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We want the "primary construtor parameter" to be indented more iff there is a following implements
clause, and they are all on separate lines.
I seem to have succeeded in that using:
builder.nestExpression();
visit(node.typeParameters);
visit(node.representation);
builder.unnest();
builder.startRule(CombinatorRule());
visit(node.implementsClause);
builder.endRule();
Someone removed the other duplicate.
33351d8
to
3c180b2
Compare
I believe the indentation formatting has been fixed (otherwise feel free to tinker more), so landing! |
Attempt to format extension types. Since they are experimental, the syntax and formatting can still change before launch,
but this should make it possible to format code containing extension type declarations.
Which will make writing tests much nicer.