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

add "Referenced Types" under References #3092

Merged
merged 8 commits into from
Mar 16, 2024

Conversation

fowl2
Copy link
Contributor

@fowl2 fowl2 commented Oct 5, 2023

Problem

Seeing the types/members referenced in an assembly reference is cumbersome.

Solution

Add a "Referenced Types" child node to each reference which contains the referenced types, including exported/forwarded types.

Future enhancements:

  • C#-ify the names
  • other children eg. ManifestResource

image
screenshot including forwarded/exported types

partially fixes #2690. It looked like trying to add "analysis" for anything that's not IEntity was going to be a bad time.

@siegfriedpammer siegfriedpammer added this to the 8.2 milestone Oct 23, 2023
@siegfriedpammer
Copy link
Member

Thank you for proposing this feature! It definitely looks interesting, some notes/comments from my side:

  • I am currently working on Support reading raw metadata blobs and Portable PDBs #3068 which will most likely require some refactoring in ILSpy's tree nodes, which will conflict with this feature. Until the refactoring is done, I won't be able to merge this.
  • The icon for exported types looks really strange, maybe we can come up with some better symbol.
  • I don't think any tree nodes should print their signature info in the tooltip, I'd prefer to do the same as we do for all other tree nodes of type definitions and members.
  • The tree node label (i.e., Text property) should use the appropriate methods of the currently selected Language implementation.

@siegfriedpammer siegfriedpammer removed this from the 8.2 milestone Nov 4, 2023
@fowl2
Copy link
Contributor Author

fowl2 commented Nov 5, 2023

Thank you for proposing this feature! It definitely looks interesting, some notes/comments from my side:

:)

I am currently working on Support reading raw metadata blobs and Portable PDBs #3068 which will most likely require some refactoring in ILSpy's tree nodes, which will conflict with this feature. Until the refactoring is done, I won't be able to merge this.

Sure.

The icon for exported types looks really strange, maybe we can come up with some better symbol.

Yeah. Maybe something like the "Type" icon with a "Shortcut" overlay? (not that I know how to implement that)

I don't think any tree nodes should print their signature info in the tooltip, I'd prefer to do the same as we do for all other tree nodes of type definitions and members.

I'm not 100% sure what you mean here, but I'll try to figure it out the next time I have a chance.

The tree node label (i.e., Text property) should use the appropriate methods of the currently selected Language implementation.

Do you mean completing the TODO inside CSharpLanguage::GetEntityName, or using a different method?

@siegfriedpammer
Copy link
Member

@fowl2 if you don't mind, I will be taking your branch and add the missing things, clean it up, etc. and push my changes here for further discussion...

@fowl2
Copy link
Contributor Author

fowl2 commented Jan 8, 2024

@siegfriedpammer sure, any way I can be help.

Have been looking into wether it would make sense to have an implementation of IMember, possibly unified with FakeMember etc. to get the correct language specific formatting and possibly the analysis features working, but I'm still thinking through any "fuzziness" with mismatches, etc.

namespace ICSharpCode.Decompiler.Metadata
{
#if !VSADDIN
public sealed class ExportedType
Copy link
Member

@siegfriedpammer siegfriedpammer Jan 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of sprinkling using SRM = System.Reflection.Metadata; everywhere I would suggest using different names for the new types.

Also, I am not sure, whether we really need them. I think that the object model for the "Referenced Types" feature should be part of the UI only.

@siegfriedpammer
Copy link
Member

Note: looking at the code, it seems it does not support showing nested type references.

@siegfriedpammer
Copy link
Member

One problem with getting nice member signatures in the tree view is that we probably want to show all members even those from assemblies that are not resolvable. So probably Language.GetEntityName based on SRM is the best we can do, without creating yet another layer doing similar things.

@siegfriedpammer
Copy link
Member

Thank you for proposing and implementing this feature. Sorry that it took so long to get this merged.

@siegfriedpammer siegfriedpammer merged commit f038055 into icsharpcode:master Mar 16, 2024
5 checks passed
@fowl2 fowl2 deleted the assemblyUsedBy branch March 18, 2024 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Used by for Assemblies
2 participants