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

Consider showing 'abstract' in quick info for abstract classes #5739

Open
DanielRosenwasser opened this issue Oct 6, 2015 · 20 comments
Open
Labels
Area-IDE Feature Request help wanted The issue is "up for grabs" - add a comment if you are interested in working on it
Milestone

Comments

@DanielRosenwasser
Copy link
Contributor

image

See microsoft/TypeScript#4610.

@aluanhaddad
Copy link

This would definitely be useful. +1

@Pilchie
Copy link
Member

Pilchie commented Oct 8, 2015

The question is what set of modifiers to show? what about static? What about readonly for fields? what about accessibility (already in the glyph we show), etc.

@Pilchie Pilchie added this to the Unknown milestone Oct 8, 2015
@Pilchie Pilchie added the help wanted The issue is "up for grabs" - add a comment if you are interested in working on it label Oct 8, 2015
@Pilchie
Copy link
Member

Pilchie commented Oct 8, 2015

Marking up for grabs, but if someone wants to pick this up, let's talk about what we want in more detail.

@DanielRosenwasser
Copy link
Contributor Author

I actually think all of those should be visible.

@dsaf
Copy link

dsaf commented Oct 9, 2015

In that case custom attributes should be shown as well #711. They can materially affect the behavior of the class and thus are as important to show as standard attributes.

@Pilchie
Copy link
Member

Pilchie commented Oct 9, 2015

And what if you're looking at a type like CSharpPackage? Do you want to see all those attributes in QuickInfo? That's why I don't think this is a simple thing to "just do".

@dsaf
Copy link

dsaf commented Oct 9, 2015

@Pilchie Yes, I want to see them:

  1. CSharpPackage is an exception rather than a rule. Even the team member commented that attributes are abused:
// TODO(DustinCa): Put all of this in CSharpPackageRegistration.pkgdef rather than using attributes
  1. How is this different from looking at a long method signature that has a dozen of type arguments and few dozen of parameters? If I will show you such an unreasonable method does it mean we need to stop showing method tooltips? Any language feature can be abused to make tooltips less useful.

  2. I don't see a problem with big tooltips. If you just navigate to class definition you are effectively looking at a big tab-tooltip that you have to close. Peek Definition is very similar to a big tooltip.

  3. Option to show attributes can be disabled by default. When I installed VS 2015 it had the "Peek Definition" turned on by default. I googled and found how to turn it off in 2 minutes: http://stackoverflow.com/questions/24433905/can-you-turn-off-peek-definition-in-visual-studio-2013

@CyrusNajmabadi
Copy link
Member

I think i'd rather have quick-info just peek at the original source code. It seems like we've got an interesting way of really thinking about things in the context of how the features were implemented in VS 20+ years ago :)

Show me the original code, in some beautiful, easy to read, manner, and then we don't really have to worry about what 'QuickInfo' looks like at all :)

@dsaf
Copy link

dsaf commented Oct 12, 2015

@CyrusNajmabadi

I think i'd rather have quick-info just peek at the original source code.

I prefer executive summary.

It seems like we've got an interesting way of really thinking about things in the context of how the features were implemented in VS 20+ years ago :)

Yeah, showing text files slightly different ways.

Show me the original code, in some beautiful, easy to read, manner, and then we don't really have to worry about what 'QuickInfo' looks like at all :).

This implies that the code is available and open-source or you are comfortable with the legal implications of decompiling on the fly (the way ReSharper does it).

@CyrusNajmabadi
Copy link
Member

or you are comfortable with the legal implications of decompiling on the fly

We already do this. It's how Metadata-as-source works.

@dsaf
Copy link

dsaf commented Oct 12, 2015

@CyrusNajmabadi Who is "we" and how is this relevant for me not willing to decompile someone's proprietary algorithm just because I want to quickly and reliably see if a method is marked with certain attributes?

@CyrusNajmabadi
Copy link
Member

Who is "we"

Roslyn/C#-ide/VB-ide.

This feature already exists. It has existed since VS2005 I believe.

how is this relevant for me not willing to decompile someone's proprietary algorithm just because I want to quickly and reliably see if a method is marked with certain attributes?

We already have to read in metadata in the first place to show you anything in QuickInfo. That's how we can tell you the name, type of symbol, and all the other information we show. I'm just saying we can consider unifying our two experiences around showing symbol information (i.e. Peek and QuickInfo). Instead of having two entirely separate features, we could try to land on just one.

@dsaf
Copy link

dsaf commented Oct 12, 2015

@CyrusNajmabadi I did not realise you are in the team. I now also understand there is a difference between reconstructing partial code from metadata and decompiling full code.

...we could try to land on just one...

That could be interesting, but it would not show e.g. all inherited attributes, just the current ones. It would be nice to scan the whole hierarchy (if present) to show the metadata. I also dislike having to press anything, hovering over for a tooltip is ideal for me.

@CyrusNajmabadi
Copy link
Member

That could be interesting, but it would not show e.g. all inherited attributes, just the current ones.

True. Though this is a problem today as well. But with a 'peek-like' view, you would ideally be able to walk up the inheritance hierarchy easily.

I also dislike having to press anything, hovering over for a tooltip is ideal for me.

Agreed. I'd still utilize the 'hover' concept. I'm just saying that we could consider using an actual view of the source (or Metadata-as-source), instead of the presentation that Quick-Info currently utilizes.

@dsaf
Copy link

dsaf commented Oct 13, 2015

I personally like the way this plug-in does it - lots of options configurable to everyone's taste:

https://github.com/MrJul/ReSharper.EnhancedTooltip

image

image

@pawchen
Copy link
Contributor

pawchen commented Oct 16, 2015

To be honest I often overlook the icon, maybe I have some difficult distinguishing different kinds of them. I understand that icons are used to save some space, but it would be nice to add some text overlay to the icon, however it's too small.

@dsaf
Copy link

dsaf commented Oct 16, 2015

@DiryBoy There is no universal graphical symbol for "abstract" or "class" or "abstract class". It's a hard problem even with more concrete concepts: http://ux.stackexchange.com/questions/1795/when-to-use-icons-vs-icons-with-text-vs-just-text-links

Luckily ReSharper and ET come to rescue:

image

@pawchen
Copy link
Contributor

pawchen commented Oct 16, 2015

@dsaf I do see an A over the left of the icon on your screenshot, that's what I'm talking about.

@AnthonyDGreen
Copy link
Contributor

@DanielRosenwasser,

Why is this information useful? How does it change your behavior knowing that it's abstract?

@DanielRosenwasser
Copy link
Contributor Author

Hey @AnthonyDGreen, it's useful to quickly understand the sort of contracts that are in place for a codebase. It also could help users from accidentally going to the definition of an abstract method (which tends to be slightly frustrating) when you're dealing with the usage for an abstract class.

@jaredpar jaredpar removed this from the Unknown milestone Nov 23, 2015
@davkean davkean added this to the Unknown milestone Dec 4, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE Feature Request help wanted The issue is "up for grabs" - add a comment if you are interested in working on it
Projects
None yet
Development

No branches or pull requests