Unused/inactive highlighting when file is not linked

[cynecx]

rust-analyzer version: 4af21ff

Before an unlinked file (or a #[cfg(FALSE)] module) just had a small diagnostic for the first line. Now the whole file gets the visually marked as "unused", which was implemented here: #17350.

The PR mentions:

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.

I agree it is less "invasive" compared to pop-up but now it's also more visually distracting (or for me straight up annoying). My workflow quite often consists of workflows where I've got unlinked files in the work-tree and having the whole file "grayed out" it not great visually speaking.

There must be a better to do this. It might be interesting to see what RustRover is doing here. But for now I've reverted this change locally, so it's not bugging me anymore.

[Veykril]

RustRover shows a banner-like thing at the top of the file I believe. VSCode doesn't really let us to do that unfortunately.

@davidbarsky @Wilfred I'll put this on your plate to figure out :) I figured people would find this annoying as that was the exact reason why the diagnostic was limited to the first few characters of the file

[RalfJung]

Yeah this definitely sounds quite annoying, e.g. when working on ui test files in rustc and Miri which are expected to be "unlinked".

[Veykril]

The only thing i can think of is to turn the status bar item yellow when having an unlinked file open, but people like to completely ignore the status bar so I don't know how useful that would be.

[lnicola]

How about a code lens at the top of the file? It might still be too easy to miss though.

[RalfJung]

I think a big part of the problem here is dealing with intentionally unlinked files. If there's a better way to avoid false warnings then it is less of a problem to make the warning more visible.

[cynecx]

I agree with Ralf here. But what can we do in the meantime until this gets figured out? I'd be in favor of reverting that PR or at the very least put it behind a ff/config.

[lnicola]

Disabling the diagnostic (rust-analyzer.diagnostics.disabled) probably works.

[cynecx]

Oh… I didn't know that setting existed 😅 Thanks!

[davidbarsky]

Yeah, I think it's reasonable that some people are annoyed by this, which is why I think should be controlled at the language/build system level, not the IDE level¹. However, that discussion isn't immediately relevant to this issue.

Short-term, I think that @lnicola's solution will work. Medium-term, what do y'all think of this: change this diagnostic to a notification and add a configuration option that allows users to clearlist certain files paths, such as those corresponding to rustc/miri's UI tests. With the newly-landed rust-analyzer.toml functionality, it'd be possible to enable this setting for all people using rust-analyzer in rustc/rust-analyzer.

Footnotes

1. We did see that new Rust users benefitted tremendously from this diagnostic, so I'm personally inclined to keep this around in some form. ↩

[Veykril]

No, this is not going to be a notification. I cannot think of something even more annoying than that. We had people screaming at us for ages because of us showing a notification when the server panicked, this would get us immense backlash if we were to do that.

[davidbarsky]

How would you feel about having this being a diagnostic by default, and behind a off-by-default setting, the ability to upgrade it to a notification? For the users Wilfred and I are responsible for, we'll make it a notification, but most rust-analyzer users won't see, let alone complain about it.

[Wilfred]

For what it's worth, #[cfg(FALSE)] hasn't changed, it has always shown inactive highlighting across the whole scope.

As @davidbarsky mentioned, this was a huge win when we measured it for our users. Rust newcomers would find that go-to-def didn't work for some files, and not realise that they needed to e.g. add mod new_file; to lib.rs.

I totally get that it's awkward when you have a deliberately untracked file, and hurts power users / exotic setups more.

The obvious options I see are:

• Adding a setting to opt-in to the old behaviour, perhaps for both untracked files and inactive cfgs.
• Adding the ability to mark a file as intentionally untracked to rust-analyzer, say a custom comment syntax. It's already possible to do cfg(rust_analyzer) I think.
• Mention the rust-analyzer.diagnostics.disabled setting in the hover text for inactive and unused files.

[davidbarsky]

RustRover shows a banner-like thing at the top of the file I believe. VSCode doesn't really let us to do that unfortunately.

To add to this, the reason I brought up the notification approach—despite users in the past being very reasonably annoyed by it!—is that the UI guidelines for VS Code suggest using a notification for this sort of thing. I would love to have a banner like RustRover does or maybe even have a modal creating a new file, but I don't know if that's a good idea, let alone possible in VS Code.

[cynecx]

Isn't it possible to color code lenses? (With decorations?) I remember seeing some extensions doing that. Even without color I'd think that this is "visible" enough (compared to the previous "short-span" behavior). But idk...

EDIT: A code lense might be somewhat convenient here because it can provide an action to add the file automatically to the parent module... 🙃

[Wilfred]

#17411 improves the hover information: it's using full sentences, and it suggests the diagnostic setting for power users.

[RalfJung]

I'm now also getting this for files that are in the module hierarchy, but RA is still loading:

That's just a plain false positive, IMO.

[Veykril]

No, this is not going to be a notification. I cannot think of something even more annoying than that. We had people screaming at us for ages because of us showing a notification when the server panicked, this would get us immense backlash if we were to do that.

I remembered earlier that we actually used to have a notification (we still have a config that toggles it), but that is nowadays disabled fully in code as it is buggy and more importantly annoying even with the setting. (buggy in part because r-a will flag any file on startup)

[Veykril]

Isn't it possible to color code lenses? (With decorations?) I remember seeing some extensions doing that. Even without color I'd think that this is "visible" enough (compared to the previous "short-span" behavior). But idk...

EDIT: A code lense might be somewhat convenient here because it can provide an action to add the file automatically to the parent module... 🙃

I don't think so? We could decorate the first line in a file and color that at best

[Veykril]

I'm now also getting this for files that are in the module hierarchy, but RA is still loading:

That's just a plain false positive, IMO.

That didn't really change either by that PR, when r-a starts up it has no crate-graph yet so it lints all files. Its just more noticable now (flagging files at start up is something we ought to not do generally).

[RalfJung]

I remembered earlier that we actually used to have a notification (we still have a config that toggles it), but that is nowadays disabled fully in code as it is buggy and more importantly annoying even with the setting. (buggy in part because r-a will flag any file on startup)

If it wasn't buggy, maybe it wouldn't be so annoying?

[Wilfred]

@RalfJung agreed on the startup case being annoying, #17415 should fix that.

[Veykril]

Does #17415 resolve this for you now?

[cynecx]

@Veykril I mean it improves things but that diagnostic, in general, is just not compatible with workflows where you have intentionally unlinked files. That's why I am currently just utilizing rust-analyzer.diagnostics.disabled. So I guess this is technically fixed for me, even though I still think the situation isn't perfect. Feel free to to close this issue then, or keep this opened for further issue tracking for anything "unlinked file" related.

EDIT: Actually on second thought, let's just close this one. It's probably best to track further improvements in a new issue tracker or something. Thanks everyone!