refactor(formatter): Refactor try_print_flat to fits_on_line
#2589
Conversation
| // print the group in "flat" mode, otherwise continue in expanded mode | ||
|
|
||
| let flat_args = args.with_print_mode(PrintMode::Flat); | ||
| // TODO replace `ElementCallQueue::default` with the real queue to also measure the rest of the document |
There was a problem hiding this comment.
I'll follow up on this TODO in #2579 Passing the rest of the document will make sure that e.g. the following snipped breaks if the ] doesn't fit on the line:
[a, b, c]
Parser conformance results on ubuntu-latestjs/262
jsx/babel
ts/babel
ts/microsoft
|
Deploying with
|
| Latest commit: |
18b7dc1
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ca04230f.tools-8rn.pages.dev |
MichaReiser
force-pushed
the
refactor/printer-try-flat-to-fits
branch
from
b134521
to
7d07d34
Compare
| // print the group in "flat" mode, otherwise continue in expanded mode | ||
|
|
||
| let flat_args = args.with_print_mode(PrintMode::Flat); | ||
| // TODO replace `ElementCallQueue::default` with the real queue to also measure the rest of the document |
There was a problem hiding this comment.
This is part of #2579 Passing in the queue will make sure that [a, b, c] only fits if the ] also fits on the line even if it's outside of the group.
There was a problem hiding this comment.
I think it would still need to allocate a clone of the current queue and not use the printer queue itself, since fits_on_line is just doing a lookahead check but shouldn't be actually dequeuing later elements
There was a problem hiding this comment.
MeasureQueue only iterates over the rest element without dequing them (read only)
tools/crates/rome_formatter/src/printer/mod.rs
Lines 814 to 825 in 4caecca
| } | ||
|
|
||
| pub fn with_incremented_indent(mut self) -> Self { | ||
| self.indent += 1; |
There was a problem hiding this comment.
Some comments have moved in this PR because with_incremented_indent only incremented the indent but didn't copy over the other fields like hard_group.
This PR brings the Printer closer to Prettier's printer by introducing a `fits_on_line` function that measures if printing this element won't exceed the line width.
This is somewhat similar to the existing `try_print_flat` implementation but differs in that sense that it also supports
measuring content that isn't printed in flat mode.
This functionality will be needed to implement alternatives because
the alternatives picks the first variant that fits on the line but
an alternative itself doesn't have to be printed in flat mode.
This PR further introduces the concept of a `GroupId` and adds the ability to use `if_group_breaks` and `if_group_fits_on_line` with a specific group id. This is necessary because this PR aligns the `fill_elements` semantic with Prettier's by introducing an implicit group for each element. The problem of doing this is that the trailing comma insertion no longer worked because the element itself fits on the line (printed in flat mode) but the outer group of the array did break:
```
group([
'[',
indent([
soft_line_break(),
fill([
[
token('2222222'),
if_group_breaks(',') // Doesn't get printed because the element fits
]
]),
soft_line_break(),
])
```
MichaReiser
force-pushed
the
refactor/printer-try-flat-to-fits
branch
from
7d07d34
to
f8ca589
Compare
|
!bench_formatter |
| /// * The first and second content fit on a single line. It prints the content and separator in flat mode. | ||
| /// * The first content fits on a single line, but the second doesn't. It prints the content in flat and the separator in expanded mode. | ||
| /// * Neither the first nor the second content fit on the line. It brings the first content and the separator in expanded mode. | ||
| fn print_fill( |
There was a problem hiding this comment.
@NicholasLYang it should be straightforward for you to change the fill semantics so that it supports custom separators. Something I understand you need for JSX. This is prettier's implementation
Make sure to also change the fits handling inside fits_on_line
Formatter Benchmark Results |
|
!bench_formatter |
Formatter Benchmark Results |
| } | ||
|
|
||
| #[cfg(target_pointer_width = "64")] |
There was a problem hiding this comment.
Checking the pointer width seems unnecessary, TextRange is internally represented as two u32 so it's size is constant for all platforms, so having this cfg just means the assert isn't included when we build for WASM
There was a problem hiding this comment.
The macro is only defined in that configuration but I guess I can remove that restrictions
There was a problem hiding this comment.
The checks don't seem to work for wasm. So I'll leave it as-is for now.
| // print the group in "flat" mode, otherwise continue in expanded mode | ||
|
|
||
| let flat_args = args.with_print_mode(PrintMode::Flat); | ||
| // TODO replace `ElementCallQueue::default` with the real queue to also measure the rest of the document |
There was a problem hiding this comment.
I think it would still need to allocate a clone of the current queue and not use the printer queue itself, since fits_on_line is just doing a lookahead check but shouldn't be actually dequeuing later elements
MichaReiser
force-pushed
the
refactor/printer-try-flat-to-fits
branch
from
4caecca
to
a4f4ca6
Compare
|
Addressed the review feedback:
|
MichaReiser
force-pushed
the
refactor/printer-try-flat-to-fits
branch
from
a4f4ca6
to
18b7dc1
Compare
MichaReiser
changed the title
try_print_flat to fits_on_line
This PR brings the Printer closer to Prettier's printer by introducing a
fits_on_linefunction that measures if printing this element won't exceed the line width.This is somewhat similar to the existing
try_print_flatimplementation but differs in the sense that it also supportsmeasuring content that isn't printed in flat mode.
This functionality will be needed to implement alternatives because
the alternatives picks the first variant that fits on the line but
an alternative itself doesn't have to be printed in flat mode.
The PR doesn't change the semantics of
fitsyet so that only fits if the group's content fit as well as the remaining content in the document before the first new line character.This PR further introduces the concept of a
GroupIdand adds the ability to useif_group_breaksandif_group_fits_on_linewith a specific group id. This is necessary because this PR aligns thefill_elementssemantic with Prettier's by introducing an implicit group for each element. The problem of doing this is that the trailing comma insertion no longer worked because the element itself fits on the line (printed in flat mode) but the outer group of the array did break: