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

feat(rome_formatter): will_break utility #2771

Merged
merged 13 commits into from
Jun 29, 2022
Merged

feat(rome_formatter): will_break utility #2771

merged 13 commits into from
Jun 29, 2022

Conversation

ematipico
Copy link
Contributor

@ematipico ematipico commented Jun 23, 2022

Summary

This PR adds two new APIs to the formatter:

  • inspect_will_break: this API creates a new buffer where the element gets written in the main buffer, but it also uses the FormatElement::will_break to track if the element written in the buffer breaks. The consumer has to call .finish() to retrieve that information. This means that the information can be retrieved only after the element is written in the buffer.
  • inspect_null: this API creates a buffer where all the formatted elements are ignored. This can be useful to compute information before the elements are written in the main buffer.

I created a couple of examples that work with the "will break" functionality.

The PR also adds a couple of bug fixes around call arguments. The checks come from prettier, and they are not aligned with it; here's the source code: https://github.com/prettier/prettier/blob/9dd761a6e491ffff3856eea47fb10b4573b351a6/src/language-js/print/call-arguments.js#L235-L244

Test Plan

cargo test and checked that the snapshots matches prettier

PR
File Based Average Prettier Similarity: 76.41%
Line Based Average Prettier Similarity: 71.98%

main
File Based Average Prettier Similarity: 76.38%
Line Based Average Prettier Similarity: 71.92%

@ematipico ematipico changed the title fix(rome_js_formatter): custom formatting for test calls feat(rome_formatter): will_break utility Jun 23, 2022
@cloudflare-pages
Copy link

cloudflare-pages bot commented Jun 23, 2022

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: e0b9756
Status: ✅  Deploy successful!
Preview URL: https://2bfeef10.tools-8rn.pages.dev
Branch Preview URL: https://feature-will-break.tools-8rn.pages.dev

View logs

@ematipico ematipico force-pushed the feature/will-break-functionality branch from 8a0c3af to 568aca5 Compare June 23, 2022 18:05
crates/rome_formatter/src/builders.rs Outdated Show resolved Hide resolved
crates/rome_formatter/src/builders.rs Outdated Show resolved Hide resolved
crates/rome_formatter/src/builders.rs Outdated Show resolved Hide resolved
MichaReiser added a commit that referenced this pull request Jun 24, 2022
This PR adds the `inspect` method to `Memoized` that allows inspecting the formatted content.

This can be useful in scenarios where some formatting must determine if some content will break to decide the best formatting.

This is different from #2771 because the buffer should be preferred in cases where the fact whatever some content breaks is only used to determine the formatting of another element because it avoids interning/memoizing the inner content.

The API shown here should be preferred when the formatting of the element itself depends on whatever its content breaks where using the `WillBreakBuffer` would need to rewind to the previous state.
MichaReiser added a commit that referenced this pull request Jun 24, 2022
This PR adds the `inspect` method to `Memoized` that allows inspecting the formatted content.

This can be useful in scenarios where some formatting must determine if some content will break to decide the best formatting.

This is different from #2771 because the buffer should be preferred in cases where the fact whatever some content breaks is only used to determine the formatting of another element because it avoids interning/memoizing the inner content.

The API shown here should be preferred when the formatting of the element itself depends on whatever its content breaks where using the `WillBreakBuffer` would need to rewind to the previous state.
MichaReiser added a commit that referenced this pull request Jun 24, 2022
This PR adds the `inspect` method to `Memoized` that allows inspecting the formatted content.

This can be useful in scenarios where some formatting must determine if some content will break to decide the best formatting.

This is different from #2771 because the buffer should be preferred in cases where the fact whatever some content breaks is only used to determine the formatting of another element because it avoids interning/memoizing the inner content.

The API shown here should be preferred when the formatting of the element itself depends on whatever its content breaks where using the `WillBreakBuffer` would need to rewind to the previous state.
MichaReiser added a commit that referenced this pull request Jun 24, 2022
This PR adds the `inspect` method to `Memoized` that allows inspecting the formatted content.

This can be useful in scenarios where some formatting must determine if some content will break to decide the best formatting.

This is different from #2771 because the buffer should be preferred in cases where the fact whatever some content breaks is only used to determine the formatting of another element because it avoids interning/memoizing the inner content.

The API shown here should be preferred when the formatting of the element itself depends on whatever its content breaks where using the `WillBreakBuffer` would need to rewind to the previous state.
MichaReiser added a commit that referenced this pull request Jun 24, 2022
This PR adds the `inspect` method to `Memoized` that allows inspecting the formatted content.

This can be useful in scenarios where some formatting must determine if some content will break to decide the best formatting.

This is different from #2771 because the buffer should be preferred in cases where the fact whatever some content breaks is only used to determine the formatting of another element because it avoids interning/memoizing the inner content.

The API shown here should be preferred when the formatting of the element itself depends on whatever its content breaks where using the `WillBreakBuffer` would need to rewind to the previous state.
@ematipico ematipico force-pushed the feature/will-break-functionality branch 3 times, most recently from 2f0b4df to 5654902 Compare June 24, 2022 13:37
Base automatically changed from feature/will-break-functionality to main June 24, 2022 13:44
MichaReiser added a commit that referenced this pull request Jun 24, 2022
This PR adds the `inspect` method to `Memoized` that allows inspecting the formatted content.

This can be useful in scenarios where some formatting must determine if some content will break to decide the best formatting.

This is different from #2771 because the buffer should be preferred in cases where the fact whatever some content breaks is only used to determine the formatting of another element because it avoids interning/memoizing the inner content.

The API shown here should be preferred when the formatting of the element itself depends on whatever its content breaks where using the `WillBreakBuffer` would need to rewind to the previous state.
@ematipico ematipico temporarily deployed to aws June 28, 2022 10:53 Inactive
@github-actions
Copy link

github-actions bot commented Jun 28, 2022

@ematipico ematipico temporarily deployed to aws June 28, 2022 11:02 Inactive
@ematipico ematipico requested review from leops and yassere June 28, 2022 11:04
@ematipico ematipico marked this pull request as ready for review June 28, 2022 11:04
@@ -176,7 +176,7 @@ impl<'buf, Context> Formatter<'buf, Context> {
impl<Context> Formatter<'_, Context> {
/// Take a snapshot of the state of the formatter
#[inline]
pub fn snapshot(&self) -> FormatterSnapshot {
pub fn state_snapshot(&self) -> FormatterSnapshot {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two functions were going in conflict with the ones created by the Buffer trait. While using them in some cases, the compiler wasn't able to compute the correct types.

@ematipico ematipico temporarily deployed to aws June 28, 2022 17:17 Inactive
@ematipico ematipico requested a review from leops June 28, 2022 17:17
@ematipico ematipico merged commit c518066 into main Jun 29, 2022
@ematipico ematipico deleted the feature/will-break branch June 29, 2022 07:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants