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

feat(rome_js_formatter): Hug function parameters #2993

Merged
merged 3 commits into from
Aug 3, 2022

Conversation

MichaReiser
Copy link
Contributor

@MichaReiser MichaReiser commented Aug 3, 2022

This PR implements the heuristic for when we want that the ( and ) hug the first parameter. This is desired when:

  • It is a test function
  • The function has a single parameter AND we want that the first token of that parameter is printed on the same line as (. This is the case for arrays and object literals OR identifiers where the type annotation is an object type

This PR further unifies the formatting logic of normal parameters and constructor parameters.

This PR does not implement the special formatting of last argument yet.

Test Plan

I manually reviewed the snapshot changes.

File Based Average Prettier Similarity: 78.10% -> 78.53%
Line Based Average Prettier Similarity: 73.19% -> 74.03%

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Aug 3, 2022

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 1e8130d
Status: ✅  Deploy successful!
Preview URL: https://6dc085d0.tools-8rn.pages.dev
Branch Preview URL: https://feat-function-parameters.tools-8rn.pages.dev

View logs

@MichaReiser MichaReiser marked this pull request as ready for review August 3, 2022 08:48
@MichaReiser
Copy link
Contributor Author

!bench_formatter

@MichaReiser MichaReiser added the A-Formatter Area: formatter label Aug 3, 2022
@github-actions
Copy link

github-actions bot commented Aug 3, 2022

Formatter Benchmark Results

group                                    main                                   pr
-----                                    ----                                   --
formatter/checker.ts                     1.01    435.2±2.75ms     6.0 MB/sec    1.00    429.1±2.19ms     6.1 MB/sec
formatter/compiler.js                    1.00    266.8±1.45ms     3.9 MB/sec    1.00    265.9±1.39ms     3.9 MB/sec
formatter/d3.min.js                      1.00    213.9±2.11ms  1254.6 KB/sec    1.00    214.4±1.59ms  1252.1 KB/sec
formatter/dojo.js                        1.00     14.3±0.05ms     4.8 MB/sec    1.02     14.6±0.06ms     4.7 MB/sec
formatter/ios.d.ts                       1.00    286.7±2.62ms     6.5 MB/sec    1.05    300.7±0.71ms     6.2 MB/sec
formatter/jquery.min.js                  1.02     58.0±0.78ms  1460.1 KB/sec    1.00     56.7±0.69ms  1491.6 KB/sec
formatter/math.js                        1.04    457.8±3.74ms  1448.4 KB/sec    1.00    442.0±1.47ms  1500.3 KB/sec
formatter/parser.ts                      1.00      9.5±0.01ms     5.1 MB/sec    1.02      9.8±0.01ms     5.0 MB/sec
formatter/pixi.min.js                    1.04    246.6±1.69ms  1822.3 KB/sec    1.00    238.0±1.82ms  1888.4 KB/sec
formatter/react-dom.production.min.js    1.01     70.8±0.72ms  1664.9 KB/sec    1.00     70.2±0.84ms  1679.9 KB/sec
formatter/react.production.min.js        1.00      3.5±0.01ms  1812.7 KB/sec    1.03      3.6±0.00ms  1759.1 KB/sec
formatter/router.ts                      1.00      7.4±0.01ms     8.3 MB/sec    1.01      7.5±0.02ms     8.1 MB/sec
formatter/tex-chtml-full.js              1.04    592.8±3.91ms  1574.1 KB/sec    1.00    570.8±4.19ms  1634.9 KB/sec
formatter/three.min.js                   1.00    281.4±2.25ms     2.1 MB/sec    1.00    281.4±1.76ms     2.1 MB/sec
formatter/typescript.js                  1.01   1807.8±8.41ms     5.3 MB/sec    1.00  1796.4±14.03ms     5.3 MB/sec
formatter/vue.global.prod.js             1.00     93.5±1.41ms  1319.8 KB/sec    1.02     95.2±1.29ms  1296.4 KB/sec

@ematipico ematipico added this to the 0.9.0 milestone Aug 3, 2022
Copy link
Contributor

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

No particular concerns, mostly questions and documentation suggestions

Base automatically changed from feature/function-formatting to main August 3, 2022 11:47
Implement the same heuristic as Prettier for grouping parameters with the return type annotation.

This ensures that a return type will break FIRST before the parameters and the parameters only break if they don't fit on the same line together with the expanded return type annotation.

This PR further unifies the formatting of methods and functions to achieve better code reuse.
@MichaReiser MichaReiser temporarily deployed to aws August 3, 2022 11:48 Inactive
@github-actions
Copy link

github-actions bot commented Aug 3, 2022

@MichaReiser MichaReiser merged commit 26be739 into main Aug 3, 2022
@MichaReiser MichaReiser deleted the feat/function-parameters branch August 3, 2022 13:16
IWANABETHATGUY pushed a commit to IWANABETHATGUY/tools that referenced this pull request Aug 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Formatter Area: formatter
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

2 participants