Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

unset all defaults in .editorconfig for *.rs #9387

Closed
wants to merge 2 commits into from

Conversation

nuke-web3
Copy link
Contributor

Opts to use rustfmt.toml behavior for rust files by ignoring in .editorconfig based on #8982

https://editorconfig.org/#supported-properties
Node.js canonical usage example

Opts to use `rustfmt.toml` behavior for rust files by ignoring in .editorconfig

https://editorconfig.org/#supported-properties
[Node.js canonical usage example](https://github.com/nodejs/node/pull/28440/files)
@nuke-web3 nuke-web3 added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Jul 19, 2021
@nuke-web3 nuke-web3 requested a review from ordian July 19, 2021 17:14
Copy link
Member

@ordian ordian left a comment

Choose a reason for hiding this comment

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

Fine by me, however, we might want to keep it as is until all the files are formatted and this is enforced by CI.

Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

I don't see any reason why we need to do this. Rustfmt formats anyway and having a good default from the beginning is good.

@nuke-web3
Copy link
Contributor Author

I only worry about conflicts: not sure all IDE decide that rustfmt overrides editorconfig, and if not, then this leaves code unformatted. Thus implying that you must you rustfmt instead of assuming they have the same or non-conflicting behavior with editorconfig.

I figure it's fine with or without, this just makes the behavior more clear IMHO.

@bkchr
Copy link
Member

bkchr commented Jul 19, 2021

The one is an editor config and the other one a formatting tool. How should the former ever overwrite the later?

@nuke-web3
Copy link
Contributor Author

From my experience, "format on save" will pick up on both of these in VSCode, I have not tested the behavior yet. I can if desired and report results.

@bkchr
Copy link
Member

bkchr commented Jul 19, 2021

Yeah please test it. However there are also people who don't use vscode ;)

@nuke-web3
Copy link
Contributor Author

Embarrassingly, this seems to be a no-op , as the behavior AFAICT is the same, including active moving to a new line based on the max line length set in .editorconfig with this unset in place.

Using "format on save" with the settings.json found here for rust-analyzer + VSCode works as expected.

"rust-analyzer.rustfmt.overrideCommand": [
  "rustup",
  "run",
  "nightly",
  "--",
  "rustfmt"
],

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants