Skip to content
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

[fmt] Option to remove semicolons #13616

Closed
Conaclos opened this issue Feb 7, 2022 · 27 comments
Closed

[fmt] Option to remove semicolons #13616

Conaclos opened this issue Feb 7, 2022 · 27 comments
Labels
deno fmt Related to the "deno fmt" subcommand or dprint suggestion suggestions for new features (yet to be agreed)

Comments

@Conaclos
Copy link

Conaclos commented Feb 7, 2022

Hi!

A lot of JavaScript and TypeScript projects don't use semicolons. Some famous style conventions such as StandardJS prohibit their use (except when needed).

I suggest to add an option useSemicolon that defaults to true and can be turned off in order to remove automatically insertable semicolons. When turned off this option set dprint's semicolon option to asi.

I think it is an option in the same vein as useTabs and indentWidth: for some people this improves code readability.

@sjmueller
Copy link

Agree with this suggestion -- Prettier started the fmt revolution with a strict philosophy on being opinionated with few options. And yet, even they have the option to turn off semicolons.

Please consider 🙏

@kitsonk kitsonk added suggestion suggestions for new features (yet to be agreed) deno fmt Related to the "deno fmt" subcommand or dprint labels Mar 27, 2022
@redxtech
Copy link

redxtech commented Apr 8, 2022

I think this would be a great addition! I'm not sure how to implement it, having no experience with the deno source, but hopefully someone who is familiar would like to take a look.

@divmgl
Copy link

divmgl commented Apr 30, 2022

+1 here. Not an annoyance right now, but it would prevent having to bring in Prettier into my project, which is a Node.js tool and not Deno.

Does the Deno team have a hard stance on semi-colons? Similar to how the Go team treats formatting?

@iuriikomarov
Copy link

+1 here. Not necessary anymore.

@nikitavoloboev
Copy link

Would really love to see this added.

@chasm
Copy link

chasm commented May 19, 2022

Dprint allows asi semicolons.

@nikitavoloboev
Copy link

Do you know how to make dprint start formatting Deno files in VSCode? I have this setup now:

{
  "deno.enable": true,
  "editor.formatOnSave": true,
  "editor.defaultFormatter": "denoland.vscode-deno"
}

I assume "editor.defaultFormatter": "denoland.vscode-deno" needs changing but to what? Path to dprint?

@nikitavoloboev
Copy link

Actually https://dprint.dev/config/ doesn't have semi setting. I think I misunderstood what you meant @chasm

@nikitavoloboev
Copy link

nikitavoloboev commented May 19, 2022

I am actually super confused. dprint says it does have semiColons option with asi. But if Deno uses dprint, why is that setting not exposed?

https://dprint.dev/plugins/typescript/config/

@chasm
Copy link

chasm commented May 19, 2022

This is my Dprint config (dprint.json) and it most definitely removes semis:

{
  "incremental": true,
  "typescript": {
    "arrowFunction.useParentheses": "force",
    "binaryExpression.operatorPosition": "sameLine",
    "bracePosition": "sameLineUnlessHanging",
    "commentLine.forceSpaceAfterSlashes": true,
    "enumDeclaration.memberSpacing": "newLine",
    "functionDeclaration.spaceBeforeParentheses": true,
    "functionExpression.spaceBeforeParentheses": false,
    "ifStatement.singleBodyPosition": "nextLine",
    "ifStatement.spaceAfterIfKeyword": true,
    "ifStatement.useBraces": "always",
    "importDeclaration.spaceSurroundingNamedImports": true,
    "indentWidth": 2,
    "lineWidth": 80,
    "locked": true,
    "newLineKind": "lf",
    "operatorPosition": "nextLine",
    "preferHanging": false,
    "quoteStyle": "preferDouble",
    "semiColons": "asi",
    "spaceSurroundingProperties": true,
    "trailingCommas": "onlyMultiLine",
    "typeAnnotation.spaceBeforeColon": false,
    "typeAssertion.spaceBeforeExpression": true,
    "useTabs": true
  },
  "json": {
    "indentWidth": 2,
    "lineWidth": 80,
    "locked": true,
    "newLineKind": "lf",
    "useTabs": true
  },
  "markdown": {
    "emphasisKind": "underscores",
    "lineWidth": 80,
    "locked": true,
    "newLineKind": "lf",
    "strongKind": "asterisks",
    "textWrap": "always"
  },
  "toml": {
    "cargo.applyConventions": true,
    "comment.forceLeadingSpace": true,
    "indentWidth": 2,
    "lineWidth": 80,
    "locked": true,
    "newLineKind": "lf",
    "useTabs": true
  },
  "includes": ["**/*.{ts,tsx,js,jsx,json,jsonc,md,toml}"],
  "excludes": [".history/**"],
  "plugins": [
    "https://plugins.dprint.dev/json-0.15.2.wasm",
    "https://plugins.dprint.dev/markdown-0.13.2.wasm",
    "https://plugins.dprint.dev/toml-0.5.4.wasm",
    "https://plugins.dprint.dev/typescript-0.68.2.wasm"
  ]
}

I have Dprint installed globally and I run the above with dprint fmt.

@taoeffect
Copy link

What do people do nowadays to use StandardJS with Deno projects? What's the recommended way to do it? Is there a community config for it?

@qgates
Copy link

qgates commented Jun 7, 2022

+1 to this FR, and to mirror dprint's semicolon options. Plenty of js/ts coding standards actively discourage routine use of semicolons and asi mode deals with edge cases where necessary.

As a side note, if deno uses dprint under the hood, why not expose dprint's complete config via deno's config file. Maybe a dprint subkey under fmt or fmt..options?

@chasm
Copy link

chasm commented Jun 7, 2022

I am using Dprint directly precisely because Deno appears to be hiding options. Not clear why Deno would do that. Is it just out of sync? Or is it deliberate?

@chasm
Copy link

chasm commented Jun 7, 2022

(That said, I also use Dprint instead of Prettier on Node apps.)

@MierenManz
Copy link

Not clear why Deno would do that.

Deno's philosophy is that having 1 way to do things is better than multiple ways to do the same thing. So that includes formatting and deno has the opinion there that it is better to be explicit about semi's rather than relying on ASI

@AnInternetTroll
Copy link
Contributor

Deno's philosophy is that having 1 way to do things is better than multiple ways to do the same thing
Since when?

@chasm
Copy link

chasm commented Jun 8, 2022

Deno's philosophy is that having 1 way to do things is better than multiple ways to do the same thing. So that includes formatting and deno has the opinion there that it is better to be explicit about semi's rather than relying on ASI.

If that's true, it's sad. It means that they've chosen to lock in their own biases rather than trusting their users to make their own decisions. And over something as trivial as ASI, too. I really hope that's not the case. It would lower my opinion of Deno and Dahl quite significantly.

@Conaclos
Copy link
Author

Conaclos commented Jun 8, 2022

@qgates, @chasm
I think that it is good thing to expose custom options. This enables to change the underlying formatter if needed. From Deno point of view, dprint is just an implementation dependency.

@nikitavoloboev
Copy link

I am currently using dprint to achieve this. But the issue still stands, the majority 99.9 % of the Deno code that's published now has not needed visual noise semicolons. Simply because it's not trivial to avoid adding them.

Hope Deno team considers adding this option.

@Conaclos
Copy link
Author

Conaclos commented Jun 8, 2022

Not clear why Deno would do that.

Deno's philosophy is that having 1 way to do things is better than multiple ways to do the same thing. So that includes formatting and deno has the opinion there that it is better to be explicit about semi's rather than relying on ASI

Thus, other options such as --options-single-quot, --options-indent-width, --options-use-tabs, and --options-line-width should be removed.

I agree that the number of options should be "minimal". However, there exists endless debates in the community (single vs double quotes, tab vs space indentation, and semicolon vs no semicolon). In these case options should be provided.

@lucacasonato
Copy link
Member

This discussion is the perfect example why we will not be introducing additional options to the built in formatter. All source code written for Deno should use the same formatting options for consistency, and to reduce this kind of discussion. In our eyes ecosystem consistency is much more important than opinions of individuals. This is also why we limit configuration options of the runtime and type checker as much as we do.

Thank you for all the comments on the issue.

@lucacasonato lucacasonato closed this as not planned Won't fix, can't repro, duplicate, stale Jun 8, 2022
@denoland denoland locked as resolved and limited conversation to collaborators Jun 8, 2022
@dsherret
Copy link
Member

Since people felt so strongly about this, Deno 1.30 has semicolon configuration https://deno.com/blog/v1.30#deno-fmt-supports-configuring-semicolons

@denoland denoland unlocked this conversation Jan 26, 2023
@taoeffect
Copy link

Woohooo!!!!! Thank you guys so much!!

@chasm
Copy link

chasm commented Jan 26, 2023

It should never have been removed in the first place, but thanks for the correction.

@dsherret
Copy link
Member

dsherret commented Jan 26, 2023

It should never have been removed in the first place, but thanks for the correction.

@chasm It was never removed because it was never added. This is the first time this functionality is available.

@chasm
Copy link

chasm commented Jan 26, 2023

@dsherret My understanding was that Deno used Dprint under the covers. It was always in Dprint, which is why I have used Dprint directly instead of Deno for formatting.

If that's correct, then in that sense it was "removed". In short, the Deno folks, as explained by Luca Casonato above, decided to leave it out, consciously and deliberately.

But it's awesome that it's in there now, so all this is water under the bridge.

@taoeffect
Copy link

@chasm don't be a d*ck just be glad they fixed it :P

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deno fmt Related to the "deno fmt" subcommand or dprint suggestion suggestions for new features (yet to be agreed)
Projects
None yet
Development

No branches or pull requests