clojure-emacs / orchard

A fertile ground for Clojure tooling
Eclipse Public License 1.0
323 stars 54 forks source link

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

Closed alexander-yakushev closed 1 month ago

alexander-yakushev commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

SGTM!

alexander-yakushev commented 1 month ago

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.