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

[inspector] Show parent class hierarchy when inspecting a class #272

Merged
merged 2 commits into from
May 30, 2024

Conversation

alexander-yakushev
Copy link
Member

This PR has two implementations: a boring one and a fun one. Let's first decide which one to continue with, and then I'll take care of the tests.

  1. The boring one simply adds the supersclass to the list of interfaces we currently show when inspecting the class. It will look like this (see APersistentVector):
image

Care is taken to sort only the interfaces (like we currently do) but the superclass will always stay on top.

  1. The fun one prints the whole hierarchy all at once. Everything is clickable, of course. Like with the boring one, the superclass is always first, then come the interfaces. Duplicate interfaces are never shown twice, each is shown only the first time it comes up. Examples:
image image

I like the fun one better because it gives much more information at a glance. You can discover with surprise that a certain class transitively implements an interface. You can immediately click the interface or class that you want. The only downside I see is that the tree steals a lot of space at the top of the inspector window, so if people primarily use class inspection to see fields/methods (which they usually do), it may be annoying having to scroll down to those sections now.

@alexander-yakushev alexander-yakushev requested a review from vemv May 30, 2024 11:45
@alexander-yakushev
Copy link
Member Author

A third option would be to show all the transitive parents but as a flat list (without indentation). Not as pretty, but then we can for example give it is a clickable list that the user can click to go inside and then inspect as a regular sequence.

A fourth option is to keep the tree but show only part of it, and then make it clickable to see hierarchy as a nested map. Kinda like datafying the whole hierarchy. Compact, but would involve many more clicks to get to the thing you want to inspect.

@vemv
Copy link
Member

vemv commented May 30, 2024

Thanks for offering options!

2. may just work. It's true that it's a lot of classes, but that particular example is probably one of the 'worst' cases and not generally representative.

My thinking is that it's useful info that allows one to further inspect stuff - hiding it would make it less likely that users would discover/inspect these pieces of info.

@alexander-yakushev alexander-yakushev changed the title [inspector] Show superclass when inspecting a class [inspector] Show parent class hierarchy when inspecting a class May 30, 2024
@alexander-yakushev
Copy link
Member Author

Cool! Going with the hierarchy then.

Side question: do you think we should include Object as a superclass? On one hand, it doesn't provide much "information" that a class extends an Object, on the other hand, it feels a bit weird to exclude it synthetically.

@alexander-yakushev
Copy link
Member Author

Actually, it serves a purpose: we currently don't show anyhow that an inspected class is a class or an interface. Having Object along the parents serves as a signal.

@vemv
Copy link
Member

vemv commented May 30, 2024

SGTM!

@alexander-yakushev alexander-yakushev force-pushed the supers branch 3 times, most recently from 04906c4 to eddb2b6 Compare May 30, 2024 13:16
@alexander-yakushev
Copy link
Member Author

Finally got the tests to pass. It is a pain to come up with an example of a class that is both interesting and stays stable across different Clojure and Java versions. Hopefully this one will last for some time.

Copy link
Member

@vemv vemv left a comment

Choose a reason for hiding this comment

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

Great work!

@alexander-yakushev alexander-yakushev merged commit 99c9acd into master May 30, 2024
58 checks passed
@alexander-yakushev alexander-yakushev deleted the supers branch May 30, 2024 13:38
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