Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

Break first binary expression group exceeding line width #2414

Closed
Tracked by #2403
MichaReiser opened this issue Apr 13, 2022 · 3 comments
Closed
Tracked by #2403

Break first binary expression group exceeding line width #2414

MichaReiser opened this issue Apr 13, 2022 · 3 comments
Labels
A-Formatter Area: formatter I-Difficult Implementation: requires deep knowledge of the tools and the problem.

Comments

@MichaReiser
Copy link
Contributor

MichaReiser commented Apr 13, 2022

Rome's formatter doesn't break the right-hand side of a binary expression to a new line if the left expression and the operator together exceed the line width.

It works as expected if the binary expression is nested. Playground

Input

aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa+ 1;

aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa+ 1 + 2;

Prettier

aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +
	1;

aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +
	1 +
	2;

Rome

aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + 1;

aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +
	1 +
	2;

Expected

Consistent formatting of binary expression regardless if a binary expression is nested or not.
Formatted source matches Prettiers output.

@ematipico
Copy link
Contributor

ematipico commented Apr 13, 2022

The issue is that when we have binary like expressions of only two items, we hard group: https://github.com/rome/tools/blob/main/crates/rome_js_formatter/src/utils/binary_like_expression.rs#L239-L246

An hard group doesn't break in this particular case. On the other hand, if we just use a group, we might bump into some issue where 1 + 1 breaks on multiple lines.

We have to have access to the width

@ematipico ematipico added the I-Difficult Implementation: requires deep knowledge of the tools and the problem. label May 5, 2022
@ematipico
Copy link
Contributor

No need to access to the width. Probably using a group instead of an hard group might fix the issue. Also using conditional content such as if_group_breaks might help to reach the solution.

Still, this is a hot path and the code is quite big, so the that's why it's difficult.

@MichaReiser
Copy link
Contributor Author

This works as expected in the latest version of Rome: Playground

@MichaReiser MichaReiser moved this to Done in Rome 2022 Jun 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Formatter Area: formatter I-Difficult Implementation: requires deep knowledge of the tools and the problem.
Projects
Status: Done
Development

No branches or pull requests

2 participants