ILSpy icon indicating copy to clipboard operation
ILSpy copied to clipboard

Navigate visible history

Open miloush opened this issue 3 months ago • 14 comments

Implements #3582 to show history items:

image

@aybe

miloush avatar Oct 11 '25 09:10 miloush

Why do we need a NavigationText property?

siegfriedpammer avatar Oct 11 '25 09:10 siegfriedpammer

Because it differs from what is in the tree, it shows full types, members including type, and assembly name for some of the folders (screenshot first item)

miloush avatar Oct 11 '25 09:10 miloush

I can also imagine it could be a method where you put current state and the text would differ on where you are (like same namespace or same assembly)

miloush avatar Oct 11 '25 09:10 miloush

I think I am fine with that. Does need to be in the SharpTreeNode base class? Also definitely needs a separator other than space...

siegfriedpammer avatar Oct 11 '25 09:10 siegfriedpammer

It's also important to use the settings-aware implementation for the node text that is used everywhere else.

siegfriedpammer avatar Oct 11 '25 09:10 siegfriedpammer

It has a default implementation returning Text so no extra work for inheritors if the entry is to be the same as in the tree. NavigationState is holding SharpTreeNode nodes so that's why I put it there.

You can see in the changed files that it follows what Text is doing. The only "new syntax" is the space for the folders. What would you prefer? >?

With that said, I don't mind using Text directly if that was preferred. I just didn't find it too helpful for navigation.

miloush avatar Oct 11 '25 09:10 miloush

Maybe put the folder in parentheses?

I am not sure. I can have a closer look when I get home. Reviewing code on the phone is not ideal. 😅

siegfriedpammer avatar Oct 11 '25 09:10 siegfriedpammer

I put the folder first and where it belongs in parentheses, looks better, see the updated screenshot.

miloush avatar Oct 11 '25 10:10 miloush

I also tried to include module name in the metadata nodes but feel free to skip that commit. Done working on this PR until there is feedback to address.

miloush avatar Oct 11 '25 13:10 miloush

One thought I had: is it possible to get an actual dropdown toolbar button in WPF? Is this only included in some community library?

siegfriedpammer avatar Oct 11 '25 15:10 siegfriedpammer

There is no built-in dropdown button in WPF. I considered creating a control for it (or retemplating combobox) but since the theme and styles are external it didn't seem worth it. We could also just have it as context menu on the normal buttons and that's it.

miloush avatar Oct 11 '25 15:10 miloush

I am playing around with your PR and something I noticed:

grafik

this doesn't seem to be very useful... I think each item should include the top-level ancestor, maybe?

siegfriedpammer avatar Dec 01 '25 16:12 siegfriedpammer

Yeah I am aware of that. I thought it is a rare if not adverse scenario where you would jump through the same nodes in different assemblies. I thought about adding the top-level info, but then all the items become cluttered with full assembly name for minimal benefit in rare cases. Next thing you come with will be same assembly names using different file locations, so I think the line needs to be drawn somewhere. (to be fair I only considered adding assembly name rather than top node text, and some of the nodes, especially in the metadata bits, do not have assembly/module associated, so it didn't really help)

This is not intended to replace user's memory. You still know what you have been doing recently so you will have an idea where the items come from.

With that said, with the NavigationText we can fine tune this easily on per node basis. For example, for namespaces we could include assembly name.

I could also do some analysis when building the history list, and if the previous item "looks the same" but has different top level, append the top level.

miloush avatar Dec 03 '25 13:12 miloush

You are right. It's probably fine the way it's now.

siegfriedpammer avatar Dec 03 '25 13:12 siegfriedpammer

I addressed what I intend to.

miloush avatar Dec 14 '25 18:12 miloush

I ran another quick test and I think we can merge it like this. Thanky you very much for your contribution!

siegfriedpammer avatar Dec 14 '25 18:12 siegfriedpammer