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

Right click variable context menu needs "Find all assignments" #38267

Closed
vsfeedback opened this issue Aug 24, 2019 · 27 comments
Closed

Right click variable context menu needs "Find all assignments" #38267

vsfeedback opened this issue Aug 24, 2019 · 27 comments
Labels
Area-IDE Developer Community The issue was originally reported on https://developercommunity.visualstudio.com Feature Request Need Design Review The end user experience design needs to be reviewed and approved.
Milestone

Comments

@vsfeedback
Copy link

This issue has been moved from a ticket on Developer Community.


Currently when you right click a variable you can choose "go to definition" which is handy and "find all references" which can have hundreds of results.

It would make it much easier if you changed "Find all References" to only show where the variable value is accessed

Console.WriteLine(myvariable);

And then added a new item "Find all assignments" which shows where the variable is assigned

myvariable = "hello";

Original Comments

Jane Wu [MSFT] on 12/16/2018, 05:52 PM:

Thank you for taking the time to provide your suggestion. We will do some preliminary checks to make sure we can proceed further. You will hear from us in about a week on our next steps.

@CyrusNajmabadi
Copy link
Member

Find-All-References will let you sort/filter by read/writes. So that should suffice for this use case.

@alexfevery
Copy link

There are 3 major classifications of variable operations. Declaration, assignment, and reference. It is illogical to have a separate menu item for one, and group the two others together. Filtering should only be reserved for special rarely used situations otherwise it unnecessarily complicates VS

@CyrusNajmabadi
Copy link
Member

Adding a new concept to represent what is already possible in VS today seems unnecessarily complicated.

Filtering should only be reserved for special rarely used situations

Why? It's a simple and easy to do. It means not having to define new features and new UI just to do what can already be done.

@hyagli
Copy link

hyagli commented Aug 26, 2019

The current usage where we can use the filter in Find-All-References is too obscure. I've been using Visual Studio for 15 years and I've learned it today by reading this thread.

@CyrusNajmabadi
Copy link
Member

Teach a man to fish.

--

We genuinely stripped down the context menu by like 75% because every team felt "it's hard to find our stuff, we'll just put it in the context menu because some users will benefit from it". It's a sure-fire way to just bog everything down.

@CyrusNajmabadi
Copy link
Member

I've been using Visual Studio for 15 years and I've learned it today by reading this thread.

Well, it wasn't in VS until just a few months ago. So it's unsurprising that you wouldn't have known about it for the last 15 years. The same would have been true of the context menu. 15 years of experience woudlnt' have helped you, since it hasn't been in the context menu the majority of that time.

@hyagli
Copy link

hyagli commented Aug 26, 2019

Well, it wasn't in VS until just a few months ago. So it's unsurprising that you wouldn't have known about it for the last 15 years.

The point is, it is a feature that is currently hard to find.

15 years of experience woudlnt' have helped you, since it hasn't been in the context menu the majority of that time.

Are you trying to help this thread or are you just trying to win an argument?

@CyrusNajmabadi
Copy link
Member

Are you trying to help this thread

I'm trying to help the thread. Part of helping is providing the thinking that went into the decisions around this. As someone who wrote the FindRefs feature several times for C#, and who had to do a lot of work and design around the context menu, I've got a ton of context here that is pretty much directly on point to the discussion being had. :)

@CyrusNajmabadi
Copy link
Member

The point is, it is a feature that is currently hard to find.

Yes. But that's not a sufficient argument to put it an easier-to-find location. Consider applying that argument to all the rest of the things in VS that are hard-to-find. Consider the outcome you might get if you did that... :)

@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented Aug 26, 2019

There are 3 major classifications of variable operations. Declaration, assignment, and reference.

Note: going back to the actual proposal. This is not how C# sees things. This definitely doesn't cover everything. For example, the majority of feedback we got in the past was that people wanted to know when something could be written-to. However, you can potential write to a variable both through assignment and through reference. This is especially true now that C# has so many ref-oriented features.

This is why FindRefs actually breaks things down in the detail needed because if you want to know about 'potential writes' you need to know about more than actual 'assignments'.

Right now i'd say this proposal seems attractive on a surface level, but it doesn't actually solve the likely problem that people would want it for. If it was implemented it would likely be the case that soon afterwards people would be complaining and asking for anothe tweak for it to handle all the interesting and important cases that do come in the language quite often.

--

Note: it's often common and attractive to think that your use-case is the use case for most users. But that more-often-than-not, turns out to not be the case. The features try to address teh top use cases, ideally in a way that can be useful for all those users, not just a particular set. In this case, we have users from all walks, who need to know all sorts of information about variables. So providing the full set of real classifications enables that in a consistent manner. it would fit your needs (assignments versus non-assignments) while also being rich enough to fit the needs of other customers making requests here (i.e. around 'potential writes').

@alexfevery
Copy link

We are talking about an action users use litterally hundreds of times when writing a piece of code.

To have to fiddle around with filters for something so commonly used would be irritating.

This is more commonly used than the majority of things that are already in the context menu.

I've never used "insert snippet" or "surround with" or "view call hierarchy" and I would guess most users have not either. If you are looking for stuff to exclude start there.

Furthermore this won't clog the context menu obviously you would use a sub menu.

Right click> find >find all instances
Right click> find >find declaration
Right click> find >find all assignments
Right click> find >find all references

"All instances" could Just show every line where the variable appears.

@CyrusNajmabadi
Copy link
Member

We are talking about an action users use litterally hundreds of times when writing a piece of code.

What are you basing that claim on?

I've never used "insert snippet" or "surround with" or "view call hierarchy" and I would guess most users have not either. If you are looking for stuff to exclude start there.

You're along is to design the product based on how you use it. We actually have the telemetry to indicate how often these other items are used. Hint: they've stuck around in the context menu precisely because they are so popular :-)

@jmarolf
Copy link
Contributor

jmarolf commented Aug 26, 2019

I think something that would auto-filter the FAR results to just reads (etc.) would be useful. I agree is @CyrusNajmabadi though that we can't just add a bunch of top level UI items. Think this is worth designing

@CyrusNajmabadi
Copy link
Member

The point is, it is a feature that is currently hard to find.

Note: by default, this is now what you see in VS:

image

This table control follows the same rules as the other table controls that VS has had for many years now. So it trivially supports things like sorting/groupin/filtering. Just hovering over the column shows this like so:

image

You can then trivially see what you can filter by like so:

image

It's then easy to just filter down the 'writes' like so:

image

IMO this is very in your face and follows the same patterns and practices the rest of VS has. It's consistent and needs no other UI. Yes, it does mean learning a bit about how all of VS works. But once you learn this, you'll have skills and knowledge that apply to the entire product, instead about learning a single-one off feature.

@jmarolf
Copy link
Contributor

jmarolf commented Aug 27, 2019

One thing we could do is not show you things like Type Argument in the list if there is none. One of my problems with the filter today is that there are always a lot things to sort through to get to the only 2 things I ever want: read and write

@hyagli
Copy link

hyagli commented Aug 27, 2019

IMO this is very in your face and follows the same patterns and practices the rest of VS has.

Could you be more rude? We can't understand you when you communicate in civilized way.

Also, as you can read from the previous posts, we know where the feature is currently.
And you had to write a long post with 4 pictures to describe how to use it.

@hyagli
Copy link

hyagli commented Aug 27, 2019

As a workaround, we could customize our own menu to get this feature.

Tools -> Customize -> Commands -> Context Menu -> Editor Context Menus | Code Window
If I insert a new menu here, I could have "Find All Assignments" in my own context menu.

image

The problem is, we need a command like "Find All References" in this commands list.

@CyrusNajmabadi
Copy link
Member

We can't understand you when you communicate in civilized way.

Which part would you like clarification on?

@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented Aug 27, 2019

One thing we could do is not show you things like Type Argument in the list if there is none. One of my problems with the filter today is that there are always a lot things to sort through to get to the only 2 things I ever want: read and write

@jmarolf: #38314

If have no problem with that. But that's just a small bugfix (Which sounds like the editor will just take care if). It's not really a feature request.

@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented Aug 27, 2019

And you had to write a long post with 4 pictures to describe how to use it.

This was in response to the idea that it wasn't easy to find this. As you could see from my post I was explicitly responding to:

The point is, it is a feature that is currently hard to find.

I'm pointing out that this information is readily visible today, and that it's easy to use the existing VS UI metaphors to slice and dice this without needing one-off commands to effectively do the same thing.

I'd be more sympathetic here if the info were not visible in any way. But as the info is there, I think the bed for a specialized command for this exact purpose is unnecessary.

Going back to the original claim that there were only 3 types of variable refs, would we need to add two additional commands here? What about if sometime wants a specialized context menu item for the other types of refs?

Why not just have the info on the find-refs window, and simply group or filter from there?

@jmarolf
Copy link
Contributor

jmarolf commented Aug 27, 2019

@CyrusNajmabadi I think filtering in the tabular-data-control is hard to discover for most people. This is a VS platform problem tho

I do think reads and writes are super useful and some less subtle way of advertising this functionality would be a good idea.

We added "go-to-implementation" to the context menu because there is really no way anyone else is ever going to find that functionality. It worked, go-to-impl is one of the most used commands in VS.

Still don't think I'm convinced it should be in the context menu, but some UX improvements here are a good idea.

@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented Aug 27, 2019

@CyrusNajmabadi I think filtering in the tabular-data-control is hard to discover for most people. This is a VS platform problem tho

I'd be interested in what Platform thinks is a suitable way to address that. It does seem like a metaphor which is prevalent in a lot of apps, so i'm curious what could be done to make it more discoverable.

We added "go-to-implementation" to the context menu because there is really no way anyone else is ever going to find that functionality.

Right. Though that was a case where there was no way to actually accomplish that task. I'm more amenable to unique commands when it doesn't feel like a subset of an existing command :)

Still don't think I'm convinced it should be in the context menu, but some UX improvements here are a good idea.

a long time ago we talked about elevating language-customiztion to the FindRefs UI. i.e. you could add buttons here to control things:

image

We could have buttons for common things like all / reads / writes so you could trivially turn on these features. This would ideally be something not baked in, but possible to configure by the actual lang plugin (i.e. TypeScript/F#/etc. might have a different set of buttons they want for filtering).

@vatsalyaagrawal vatsalyaagrawal added the Developer Community The issue was originally reported on https://developercommunity.visualstudio.com label Sep 3, 2019
@vatsalyaagrawal
Copy link
Contributor

related to #25994

@vatsalyaagrawal vatsalyaagrawal added Feature Request Need Design Review The end user experience design needs to be reviewed and approved. labels Sep 3, 2019
@vatsalyaagrawal vatsalyaagrawal added this to the Backlog milestone Sep 3, 2019
@jmarolf
Copy link
Contributor

jmarolf commented Sep 3, 2019

We could have buttons for common things like all / reads / writes so you could trivially turn on these features.

This sounds like a good idea. Placing things in the results tool window would be a good place to start and track how often this feature is discovered.

Now if only we could improve discover-ability on find-all-references for literal expressions...

@vatsalyaagrawal
Copy link
Contributor

Design meeting notes:

We need more customer development to find the right solution. Some questions that came up are:
Is peek definition #1 things folks do in this menu?

  • What is the current situation with menus? What are the numbers of context menu usage? High/Low usage can help drive the right solution here.
  • Our menus are too big, can we reduce those down?
  • Can filter button be always there (not hidden)?
  • Can we have dedicated (may be context aware) commands like, find, move?
  • Can we surface/alert users about features that they have not used/might not know about? May be we can use the new feature window or something similar.

@CyrusNajmabadi
Copy link
Member

Closing out. This would need to go through editor/platform. The data roslyn is rpoviding (and would continue to provide through LSP) contains what is necessary to feed any UI improvements here.

@CyrusNajmabadi CyrusNajmabadi closed this as not planned Won't fix, can't repro, duplicate, stale Nov 1, 2022
@jmarolf
Copy link
Contributor

jmarolf commented Nov 1, 2022

@CyrusNajmabadi as we are closing this out can we "reactivate" the original feedback item and move it to the vs platform team?

@sharwell sharwell moved this to Need Proposal in IDE: Design review Aug 22, 2023
@CyrusNajmabadi CyrusNajmabadi moved this from Need Proposal to Complete in IDE: Design review Oct 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE Developer Community The issue was originally reported on https://developercommunity.visualstudio.com Feature Request Need Design Review The end user experience design needs to be reviewed and approved.
Projects
Status: Complete
Development

No branches or pull requests

7 participants