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

fix: Highlight unlinked files consistently with inactive files #17350

Merged
merged 1 commit into from
Jun 6, 2024

Conversation

Wilfred
Copy link
Contributor

@Wilfred Wilfred commented Jun 6, 2024

Currently, rust-analyzer highlights the entire region when a cfg is inactive (e.g. #[cfg(windows)] on a Linux machine). However, unlinked files only highlight the first three characters of the file.

This was introduced in #8444, but users have repeatedly found themselves with no rust-analyzer support for a file and unsure why (see e.g. #13226 and the intentionally prominent pop-up added in PR #14366).

(Anecdotally, we see this issue bite our users regularly, particularly people new to Rust.)

Instead, highlight the entire inactive file, but mark it as all as unused. This allows users to hover and run the quickfix from any line.

Whilst this is marginally more prominent, it's less invasive than a pop-up, and users do want to know why they're getting no rust-analyzer support in certain files.

Before (note the subtle grey underline is only at the beginning of the first line):

Screenshot 2024-06-05 at 5 41 17 PM

After (user can hover and fix from any line):

Screenshot 2024-06-05 at 5 42 13 PM

Currently, rust-analyzer highlights the entire region when a `cfg` is
inactive (e.g. `#[cfg(windows)]` on a Linux machine). However,
unlinked files only highlight the first three characters of the file.

This was introduced in rust-lang#8444, but users have repeatedly found
themselves with no rust-analyzer support for a file and unsure
why (see e.g. rust-lang#13226 and the intentionally prominent pop-up added in
PR rust-lang#14366).

(Anecdotally, we see this issue bite our users regularly, particularly
people new to Rust.)

Instead, highlight the entire inactive file, but mark it as all as
unused. This allows users to hover and run the quickfix from any line.

Whilst this is marginally more prominent, it's less invasive than a
pop-up, and users do want to know why they're getting no rust-analyzer
support in certain files.
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 6, 2024
@Wilfred Wilfred marked this pull request as ready for review June 6, 2024 00:55
@Veykril
Copy link
Member

Veykril commented Jun 6, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Jun 6, 2024

📌 Commit fcecbc5 has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Jun 6, 2024

⌛ Testing commit fcecbc5 with merge 202359d...

@bors
Copy link
Contributor

bors commented Jun 6, 2024

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

@bors bors merged commit 202359d into rust-lang:master Jun 6, 2024
11 checks passed
@davidbarsky davidbarsky deleted the mark_missing_file_unused branch June 6, 2024 15:44
Wilfred added a commit to Wilfred/rust-analyzer that referenced this pull request Jun 13, 2024
This partially reverts rust-lang#17350, based on the feedback in rust-lang#17397.

If we don't have an autofix, it's more annoying to highlight the whole line.
This heuristic fixes the diagnostic overwhelming the user during startup.
bors added a commit that referenced this pull request Jun 19, 2024
fix: Only show unlinked-file diagnostic on first line during startup

This partially reverts #17350, based on the feedback in #17397.

If we don't have an autofix, it's more annoying to highlight the whole file. This autofix heuristic fixes the diagnostic being overwhelming during startup.
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.

4 participants