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

feat(rome_js_formatter): Indent Doc comments #3129

Merged
merged 5 commits into from
Aug 30, 2022
Merged

Conversation

MichaReiser
Copy link
Contributor

@MichaReiser MichaReiser commented Aug 29, 2022

This PR adds support for indenting js doc comments.

/*
     * a comment
     */

Becomes

/*
 * a comment
 *

The PR introduces a new LeadingCommentRule type on the CstFormatContext that is used to format any leading comment. This allows languages to implement formatting of a comment's content, something that wasn't possible before.

Tests

Average compatibility: 83.70 -> 84.04
Compatible lines: 80.79 -> 81.22

@cloudflare-workers-and-pages
Copy link

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

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: d89d467
Status: ✅  Deploy successful!
Preview URL: https://9903a1bb.tools-8rn.pages.dev
Branch Preview URL: https://feat-doc-comments.tools-8rn.pages.dev

View logs

Base automatically changed from feat/parentheses-transform to main August 29, 2022 15:13
@MichaReiser MichaReiser temporarily deployed to aws August 29, 2022 15:37 Inactive
Comment on lines +90 to +119
#[derive(Eq, PartialEq, Copy, Clone, Debug, Default)]
pub struct JsCommentStyle;

impl CommentStyle<JsLanguage> for JsCommentStyle {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved this from context.rs

CONNECTING: Object.freeze({ kind: "CONNECTING" }),
NOT_CONNECTED: Object.freeze({ kind: "NOT_CONNECTED" }),
});
-
-/* A comment */
-/**
- * A type that can be written to a buffer.
+/* A comment */ /**
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Prettier inserts a line break here. We aren't able to do this yet because Prettier only does so if the doc is a trailing comment of a node. We'll only have this distinction (at least in a cheap way) after the comments refactor.

The only difference is that the block comments start on the same line. A user can fix this by manually inserting a line break, which is what they also have to do for other block comments that start on the same line.

@MichaReiser MichaReiser marked this pull request as ready for review August 29, 2022 15:37
@github-actions
Copy link

github-actions bot commented Aug 29, 2022

@MichaReiser MichaReiser requested a review from a team August 29, 2022 15:57
@MichaReiser MichaReiser temporarily deployed to aws August 29, 2022 15:57 Inactive
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.

You should run cargo codegen-bindings, that will update the types with comments.

@MichaReiser MichaReiser temporarily deployed to aws August 29, 2022 17:19 Inactive
@MichaReiser MichaReiser temporarily deployed to aws August 29, 2022 17:23 Inactive
@rome rome deleted a comment from github-actions bot Aug 29, 2022
@MichaReiser
Copy link
Contributor Author

!bench_formatter

@github-actions
Copy link

Formatter Benchmark Results

group                                    main                                   pr
-----                                    ----                                   --
formatter/checker.ts                     1.00    487.4±5.91ms     5.3 MB/sec    1.01    492.7±5.93ms     5.3 MB/sec
formatter/compiler.js                    1.01    278.6±2.46ms     3.8 MB/sec    1.00    275.3±3.33ms     3.8 MB/sec
formatter/d3.min.js                      1.02    234.7±2.10ms  1143.4 KB/sec    1.00    229.9±2.10ms  1167.2 KB/sec
formatter/dojo.js                        1.00     13.8±0.03ms     5.0 MB/sec    1.00     13.9±0.01ms     4.9 MB/sec
formatter/ios.d.ts                       1.01    292.7±1.26ms     6.4 MB/sec    1.00    289.9±1.54ms     6.4 MB/sec
formatter/jquery.min.js                  1.00     58.1±1.01ms  1455.9 KB/sec    1.03     60.0±1.48ms  1411.5 KB/sec
formatter/math.js                        1.01    466.3±5.51ms  1422.0 KB/sec    1.00    461.4±3.36ms  1437.2 KB/sec
formatter/parser.ts                      1.00      9.3±0.01ms     5.3 MB/sec    1.01      9.3±0.01ms     5.2 MB/sec
formatter/pixi.min.js                    1.03    266.6±4.33ms  1685.6 KB/sec    1.00    257.8±2.18ms  1743.6 KB/sec
formatter/react-dom.production.min.js    1.00     72.0±1.06ms  1637.1 KB/sec    1.04     75.1±1.32ms  1569.0 KB/sec
formatter/react.production.min.js        1.00      3.4±0.00ms  1830.6 KB/sec    1.01      3.5±0.02ms  1812.1 KB/sec
formatter/router.ts                      1.00      7.1±0.01ms     8.6 MB/sec    1.02      7.3±0.19ms     8.4 MB/sec
formatter/tex-chtml-full.js              1.00    588.4±3.71ms  1585.9 KB/sec    1.01    591.6±3.41ms  1577.4 KB/sec
formatter/three.min.js                   1.00    294.8±2.39ms  2039.6 KB/sec    1.00    294.8±5.18ms  2039.5 KB/sec
formatter/typescript.js                  1.01  1851.2±11.46ms     5.1 MB/sec    1.00   1824.3±7.69ms     5.2 MB/sec
formatter/vue.global.prod.js             1.00     97.4±1.61ms  1267.0 KB/sec    1.03    100.5±1.39ms  1227.0 KB/sec

crates/rome_js_formatter/src/comments.rs Outdated Show resolved Hide resolved
crates/rome_js_formatter/src/comments.rs Outdated Show resolved Hide resolved
crates/rome_js_formatter/src/comments.rs Show resolved Hide resolved
@MichaReiser MichaReiser temporarily deployed to aws August 30, 2022 09:24 Inactive
This PR adds support for indenting js doc comments.

```javascript
/*
     * a comment
     */
```

Becomes
```javascript
/*
 * a comment
 *
```

The PR introduces a new `LeadingCommentRule` type on the `CstFormatContext` that is used to format any leading comment. This allows languages to implement formatting of a comment's content, something that wasn't possible before.

## Tests

**Average compatibility**: 83.70 -> 84.04

<details>
	<summary>Definition</summary>

	$$average = \frac\{\sum_{file}^\{files}compatibility_\{file}}\{files}$$
</details>

**Compatible lines**: 80.79 -> 81.22
@MichaReiser MichaReiser temporarily deployed to aws August 30, 2022 09:26 Inactive
@MichaReiser MichaReiser merged commit b08b5e2 into main Aug 30, 2022
@MichaReiser MichaReiser deleted the feat/doc-comments branch August 30, 2022 09:49
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.

2 participants