-
-
Notifications
You must be signed in to change notification settings - Fork 105
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
Regression in 2.6.0 #683
Comments
Thank you for creating this issue! However, I believe this is intended behavior. At least, it mimics Prettier's own formatting for a similar code snippet in TypeScript, which is what most (if not all) of the formatting decisions in #632 were based upon. Prettier Java example: class Example {
public static final MethodDescriptor<ByteBuf, ByteBuf> NEW_TUNNEL_METHOD = MethodDescriptor.newBuilder(
ByteBufMarshaller.INSTANCE,
ByteBufMarshaller.INSTANCE
)
.setFullMethodName(TUNNEL_SERVICE + "/new")
.setType(MethodDescriptor.MethodType.BIDI_STREAMING)
.build();
} Prettier TypeScript example: class Example {
public static readonly NEW_TUNNEL_METHOD: MethodDescriptor<ByteBuf, ByteBuf> = MethodDescriptor.newBuilder(
ByteBufMarshaller.INSTANCE,
ByteBufMarshaller.INSTANCE,
)
.setFullMethodName(TUNNEL_SERVICE + "/new")
.setType(MethodDescriptor.MethodType.BIDI_STREAMING)
.build();
} Because of that, I'm doubtful that we will revert this change, as we're trying our best to keep Prettier Java aligned with Prettier's own formatting decisions, specifically in TypeScript (which tends to be the "native" language most similar to Java). |
I'll be honest tho, the current actual behavior looks noticeable worse than what we'd expect. |
@jtkiesel ah, thanks, I missed that! I wonder if it would be considered bad for TypeScript too and is just yet to be fixed. Otherwise, I agree with @lppedd that it looks like a black sheep of otherwise beautifully formatted code. Would you be open to reconsider the current behavior (even if it means slightly diverging from TS, at least until they reconsider it too)? I do agree and accept all the opinionatedness of prettier-java except this one that does look like a bug in the formatter and does not match any other formatting logic in the rest of code 😅 |
Prettier-Java 0.6.0
# Options (if any): --print-width 120
It looks like #632 , despite being a great improvement overall, introduced a regression when the first method call needs to be multi-line due to long arguments, leading to a closing
)
misaligned.Input:
Output:
Expected behavior:
Alternative expected behavior:
See bsideup/grpc-bidi@bd35134
The text was updated successfully, but these errors were encountered: