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

Rename checkOnSave settings to check #13799

Merged
merged 2 commits into from
Jan 9, 2023
Merged

Conversation

Veykril
Copy link
Member

@Veykril Veykril commented Dec 20, 2022

Now that flychecks can be triggered without saving the setting name doesn't make that much sense anymore. This PR renames it to just check, but keeps checkOnSave as the enabling setting.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 20, 2022
@lnicola
Copy link
Member

lnicola commented Dec 20, 2022

Sounds like a good idea to be consistent about this. I don't think the more obscure name is a problem.

@flodiebold
Copy link
Member

I don't know, flycheck is a term from Emacs which can make this kind of confusing if people search for the term, and it also doesn't really tell you anything. (Also, AIUI it's called "flycheck" because it checks on the fly while typing, so the name makes just as little sense as "check on save" anymore.) I would be ok with just check; if we want to be clearer about it, maybe externalCheck or something in that vein would be better?

@Veykril
Copy link
Member Author

Veykril commented Dec 20, 2022

Oh okay, let's not use flycheck publicly if it clashes with terminology from emacs, that does sound like a bad choice. externalCheck sounds good, though maybe check itself really would be fine as well, since checkOnSave technically isn't different in that regard.

@Veykril Veykril changed the title Rename checkOnSave settings to flycheck Rename checkOnSave settings to check Jan 9, 2023
@Veykril Veykril marked this pull request as ready for review January 9, 2023 13:17
f.splitn(2, '_').next().unwrap()
}
assert!(key(f1) <= key(f2), "wrong field order: {f1:?} {f2:?}");
}
Copy link
Member Author

Choose a reason for hiding this comment

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

We already assert this via a separate test

@Veykril
Copy link
Member Author

Veykril commented Jan 9, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Jan 9, 2023

📌 Commit d2bb62b has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Jan 9, 2023

⌛ Testing commit d2bb62b with merge fd300ee...

@bors
Copy link
Contributor

bors commented Jan 9, 2023

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing fd300ee to master...

@bors bors merged commit fd300ee into rust-lang:master Jan 9, 2023
@Veykril Veykril deleted the flycheck branch January 9, 2023 15:59
qryxip added a commit to qryxip/dotfiles that referenced this pull request Jan 17, 2023
lukas-code added a commit to lukas-code/rustc-dev-guide that referenced this pull request Jan 18, 2023
@kylebarron
Copy link

Just a note that I think this broke any configuration users had already set based on checkOnSave. I had set checkOnSave.command to clippy and that didn't automatically get updated to check.command when updating to this release

@lnicola
Copy link
Member

lnicola commented Feb 7, 2023

@kylebarron the server still understands the old preference, even if it shows up as unused in Code.

@kylebarron
Copy link

For me when I updated to the latest rust-analyzer version, clippy-on-save stopped working entirely 🤷‍♂️. It wasn't until I switched checkOnSave.command to check.command until it started working again. It's also possible I did something wrong though!

@lnicola
Copy link
Member

lnicola commented Feb 7, 2023

Idle musing: I wonder if we should add a "Run Clippy on crate/workspace" to the existing "Rust-Analyzer: Run", as an alternative to setting Clippy as the check command.

@lnicola
Copy link
Member

lnicola commented Feb 8, 2023

@kylebarron FWIW, I can't reproduce that:

image

@kylebarron
Copy link

🤷‍♂️. I probably did something wrong! sorry for the noise!

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request May 1, 2023
`checkOnSave` settings were renamed to `check` in
rust-lang/rust-analyzer#13799

This allows VS Code IntelliSense to show a description on hover in
the settings.json file instead of "Unknown Configuration Setting".
The old setting still works, so this doesn't change other
behaviour.

Differential Revision: https://phabricator.services.mozilla.com/D176792
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request May 2, 2023
`checkOnSave` settings were renamed to `check` in
rust-lang/rust-analyzer#13799

This allows VS Code IntelliSense to show a description on hover in
the settings.json file instead of "Unknown Configuration Setting".
The old setting still works, so this doesn't change other
behaviour.

Differential Revision: https://phabricator.services.mozilla.com/D176792

UltraBlame original commit: 99a6ee7ee5e45a34fee26c5c8c9ae4ba8215a23c
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request May 2, 2023
`checkOnSave` settings were renamed to `check` in
rust-lang/rust-analyzer#13799

This allows VS Code IntelliSense to show a description on hover in
the settings.json file instead of "Unknown Configuration Setting".
The old setting still works, so this doesn't change other
behaviour.

Differential Revision: https://phabricator.services.mozilla.com/D176792

UltraBlame original commit: 99a6ee7ee5e45a34fee26c5c8c9ae4ba8215a23c
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request May 2, 2023
`checkOnSave` settings were renamed to `check` in
rust-lang/rust-analyzer#13799

This allows VS Code IntelliSense to show a description on hover in
the settings.json file instead of "Unknown Configuration Setting".
The old setting still works, so this doesn't change other
behaviour.

Differential Revision: https://phabricator.services.mozilla.com/D176792

UltraBlame original commit: 99a6ee7ee5e45a34fee26c5c8c9ae4ba8215a23c
jamienicol pushed a commit to jamienicol/gecko that referenced this pull request May 3, 2023
`checkOnSave` settings were renamed to `check` in
rust-lang/rust-analyzer#13799

This allows VS Code IntelliSense to show a description on hover in
the settings.json file instead of "Unknown Configuration Setting".
The old setting still works, so this doesn't change other
behaviour.

Differential Revision: https://phabricator.services.mozilla.com/D176792
axelkar added a commit to axelkar/rp2040-project-template that referenced this pull request Oct 15, 2024
9names pushed a commit to rp-rs/rp2040-project-template that referenced this pull request Nov 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants