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

VS: fix unwanted navigation on hover #17592

Merged
merged 8 commits into from
Aug 26, 2024
Merged

Conversation

majocha
Copy link
Contributor

@majocha majocha commented Aug 22, 2024

Don't show the window, return a nav item instead.

Fixes #17230.

Copy link
Contributor

github-actions bot commented Aug 22, 2024

❗ Release notes required


✅ Found changes and release notes in following paths:

Change path Release notes path Description
vsintegration/src docs/release-notes/.VisualStudio/17.12.md

@majocha
Copy link
Contributor Author

majocha commented Aug 22, 2024

@vzarytovskii I don't have much time to test it at the moment, but that's what I'd try.

@vzarytovskii
Copy link
Member

vzarytovskii commented Aug 23, 2024

I am on leave now and can't test it, but unsure if we return the nav item without showing it, Roslyn will understand it and will switch focus, if it's not a part of its original workspace. And we can't add it to current workspace, since it might break checking. Maybe we can try adding it to miscellaneous files namespace, but then highlighting might break.

@majocha
Copy link
Contributor Author

majocha commented Aug 23, 2024

No worries :) I just tested it. Looks like with this change navigation to metadata still works. I can F12 or ctrl+click and I get metadata source with typecheck and semantic colors.

I think we don't need to use current workspace. Roslyn creates and uses a separate workspace for metadata as source in their own implementation:

https://sourceroslyn.io/#Microsoft.CodeAnalysis.Features/MetadataAsSource/MetadataAsSourceFileService.cs,96

@majocha majocha changed the title VS: Attempt to fix unwanted navigation on hover VS: fix unwanted navigation on hover Aug 23, 2024
@majocha majocha marked this pull request as ready for review August 23, 2024 10:30
@majocha majocha requested a review from a team as a code owner August 23, 2024 10:30
@T-Gro
Copy link
Member

T-Gro commented Aug 26, 2024

/azp run

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@T-Gro T-Gro enabled auto-merge (squash) August 26, 2024 08:42
@T-Gro
Copy link
Member

T-Gro commented Aug 26, 2024

Tested locally.

Desired navigation to external metadata still works.
Undesired navigation no longer happens to me.

Thinking about it, I was getting it during the summer as well, but was contributing it to my fat fingers.
The typical use case happening to me was when selecting a longIdent with mouse, CTRL+C for copying, which sometimes jumped me to decompiled source.

(now I know that it only happened when I was copypasting external symbols, and my mouse pointer was above the text).

@vzarytovskii
Copy link
Member

Tested locally.

Desired navigation to external metadata still works.

Undesired navigation no longer happens to me.

Thinking about it, I was getting it during the summer as well, but was contributing it to my fat fingers.

The typical use case happening to me was when selecting a longIdent with mouse, CTRL+C for copying, which sometimes jumped me to decompiled source.

(now I know that it only happened when I was copypasting external symbols, and my mouse pointer was above the text).

Yeah, sorry, it was my hotfix, haven't crossed my mind that hovering can lead to calling definitions API

@Thorium
Copy link
Contributor

Thorium commented Oct 9, 2024

Still happens to me in VS2022 17.12 Beta...

@vzarytovskii
Copy link
Member

Inserted month ago probably means it's in p3 or release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
5 participants