-
Notifications
You must be signed in to change notification settings - Fork 5.6k
feat(fmt): add fmt options #28946
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(fmt): add fmt options #28946
Conversation
The fixture files |
The schema should be updated as well. deno/cli/schemas/config-file.v1.json Lines 369 to 453 in 0bb1665
|
@petamoriken Thanks! Updated |
Signed-off-by: Yoshiya Hinosawa <[email protected]>
@dsherret PTAL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but look into the defaults as I think some of them are wrong.
Cargo.toml
Outdated
@@ -56,7 +56,8 @@ deno_ast = { version = "=0.46.5", features = ["transpiling"] } | |||
deno_core = { version = "0.343.0" } | |||
|
|||
deno_cache_dir = "=0.18.0" | |||
deno_config = { version = "=0.52.0", features = ["workspace"] } | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: remove blank line?
cli/schemas/config-file.v1.json
Outdated
}, | ||
"operatorPosition": { | ||
"description": "Where to place the operator for expressions that span multiple lines in JavaScript and TypeScript.", | ||
"default": "nextLine", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you verify these defaults are correct? I think some are wrong like this one: https://github.com/dprint/dprint-plugin-typescript/blob/936cf945ad9f0e69bb31cffe1c85b1cf991fe56e/src/configuration/builder.rs#L54
This is actually the default we should have selected, but we did sameLine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I was using dprint's default. Thanks!
Updated in ba93914
"singleBodyPosition": "nextLine", | ||
"nextControlFlowPosition": "nextLine", | ||
"trailingCommas": "never", | ||
"operatorPosition": "sameLine", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe try with the non-default
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed this to "nextLine" b16ba74
closes #26874
This PR adds 14 new options from dprint-plugin-typescript:
related: denoland/deno_config#162
remaining: