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

minor: Don't show layout info when hovering on locals #17533

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Wilfred
Copy link
Contributor

@Wilfred Wilfred commented Jul 2, 2024

This is useful when the cursor is on the type name, but layout information tends to distract when the cursor is just on a local variable of that type.

Fixes #17096

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 2, 2024
This is useful when the cursor is on the type name, but layout
information tends to distract when the cursor is just on a local
variable of that type.

Fixes rust-lang#17096
@Wilfred Wilfred marked this pull request as ready for review July 2, 2024 23:27
@Veykril
Copy link
Member

Veykril commented Jul 3, 2024

That doesn't address the issue. I like the layout info on locals a lot in fact as it is the easiest way to check the size of some arbitrary instantiated type.

As that linked issue states, Hover should only show type layout when hovering the definition, not a reference. This just removes hovers for locals though which is something completely different.

@Veykril
Copy link
Member

Veykril commented Jul 3, 2024

To add to this, we should differentiate here

match IdentClass::classify_node(sema, &node)? {
// It's better for us to fall back to the keyword hover here,
// rendering poll is very confusing
IdentClass::Operator(OperatorClass::Await(_)) => None,
IdentClass::NameRefClass(NameRefClass::ExternCrateShorthand {
decl,
..
}) => Some(vec![(Definition::ExternCrateDecl(decl), None, node)]),
class => Some(
multizip((class.definitions(), iter::repeat(None), iter::repeat(node)))
.collect::<Vec<_>>(),
),
}

If we deal with a IdentClass::NameClass we should render the layout info, otherwise we should omit it

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.

Hover should only show type layout when hovering the definition, not a reference
3 participants