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 TypeDefinitionDocuments to custom debug information data #2578

Merged

Conversation

davidwengier
Copy link
Contributor

For .NET 6/C# 10 we added a new CustomDebugInformation type for linking a TypeDef entry to a Document entry, in dotnet/roslyn#56278
This updates ILSpy to correctly display the information.

Before:
image

After:
image

The 02 in the value is correct, it refers to the 2nd row of the Document table.

Sorry I took so long to submit this PR :)

@siegfriedpammer
Copy link
Member

Thank you for your contribution! Just one question: wouldn't it be better to show the contents of the Document table entry as detail view?

@siegfriedpammer siegfriedpammer self-assigned this Dec 13, 2021
@davidwengier
Copy link
Contributor Author

Certainly can do that if you'd prefer, it wasn't clear to me whether the detail view should show just the decoded value, or do more. I will update

@siegfriedpammer
Copy link
Member

You are of course right. Now I am no longer sure whether adding that information to the details view is a good idea. Maybe it will cause confusion, as one might think that all of the information is present in the blob.

Probably just add a link to the output, which will open the Documents table in a new tab?

Would you want me to add this or do you want to have a look yourself?

@davidwengier
Copy link
Contributor Author

Is there any example where links are used to navigate between nodes now? I couldn't see anything from a quick look through the app.

In general I think more linking would be great though, for things like values in the "Document" field of the MethodDebugInformation table, or the Parent column of any metadata tables etc. Obviously not as part of this PR though :)

@davidwengier
Copy link
Contributor Author

davidwengier commented Dec 14, 2021

Showing the document record is not terrible though:
image

@siegfriedpammer
Copy link
Member

Is there any example where links are used to navigate between nodes now? I couldn't see anything from a quick look through the app.

In general I think more linking would be great though, for things like values in the "Document" field of the MethodDebugInformation table, or the Parent column of any metadata tables etc. Obviously not as part of this PR though :)

See https://github.com/icsharpcode/ILSpy/blob/master/ILSpy/Metadata/GoToTokenCommand.cs#L39 for an example. I can take a closer look tomorrow.

@davidwengier
Copy link
Contributor Author

davidwengier commented Dec 14, 2021

See https://github.com/icsharpcode/ILSpy/blob/master/ILSpy/Metadata/GoToTokenCommand.cs#L39 for an example. I can take a closer look tomorrow.

😮

I had no idea that context menu existed!

@siegfriedpammer
Copy link
Member

DataGridHyperlinkColumn would probably be a good idea to use for all fields that reference other tables.

@siegfriedpammer
Copy link
Member

I have prototyped this... the result is quite good, I think. What do you think?

hyperlinks

@davidwengier
Copy link
Contributor Author

I have prototyped this... the result is quite good, I think. What do you think?

Love it!

@siegfriedpammer siegfriedpammer merged commit f09deb4 into icsharpcode:master Dec 18, 2021
@siegfriedpammer
Copy link
Member

Thank you very much for your contribution!

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.

2 participants