-
Notifications
You must be signed in to change notification settings - Fork 189
Ensure rustfmt is invoked without edition parameter #571
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
Conversation
6873c6f to
b902906
Compare
|
Failed to set assignee to
|
636039f to
0a5c8f5
Compare
psibi
left a comment
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 apart from minor comments, I like the tests!
d617d50 to
fcab951
Compare
|
@apiraino Missed this, one more change: Can you also update the changelog: https://github.com/rust-lang/rust-mode/blob/master/Changelog.md ? |
One more thing about this change. As I mentioned in the opening comment, do you think this qualifies as a breaking change for users? I mean, people could start seeing their code being formatted differently. If I understand how MELPA publishing works, this commit will be automatically picked up by them once merged, correct? Do you do some tagging/releases for other package repositories that don't stay on top of trunk commits - like Elpa? |
fcab951 to
485d97f
Compare
485d97f to
f2c0802
Compare
Yes
Yes. Although Melpa stable won't. Although very few people actually use MELPA stable.
We do it. Although we wait for quite a bit and for lots of change to accumulate. The last release was done a year ago because we want to release support with tree-sitter: https://github.com/rust-lang/rust-mode/releases/tag/1.0.6 |
Fixes #570
rustfmtshould probably be invoked without defaulting to any--editionparameter as it collides with a user configuration in arustfmt.tomlor with the rustfmt default (2015 edition at the time of this patch).This patch sets
rust-rustfmt-switchesto nil so in a sense, I think this could be a breaking change?I've added a test to ensure that it stays so. The test is a bit flaky, it doesn't use a rutfmt.toml so it's not completely isolated. But I'm an Emacs newbie, so 🤷
Happy to receive feedback. Thanks for the review!
r? @psibi