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

Ability to sort symbols in the diff view #39

Open
ribbanya opened this issue Jan 29, 2024 · 3 comments
Open

Ability to sort symbols in the diff view #39

ribbanya opened this issue Jan 29, 2024 · 3 comments

Comments

@ribbanya
Copy link
Contributor

I was thinking it'd be useful to be able to sort symbols by...

  • Offset (the current behavior)
  • Total symbol size
  • Unmatched bytes remaining (or maybe remaining match percentage? I think byte count would be more useful though)
  • Alphabetical by name (for finding symbols quickly)

For selecting sort order, I think either a flyout menu from Diff Options (like Recent Projects), or a popup (like Algorithm), would suffice. It should override the project settings as it's a user-level preference. The user selects one sort method, and can select it again for reverse order. All sections are sorted by the same criterion.

Also, it seems like this could replace Reverse function order by just being Offset (descending), but I'm not very familiar with the use case of the existing option.

Given approval of the idea and some pointers on implementation, I wouldn't mind implementing it.

@encounter
Copy link
Owner

encounter commented Jan 30, 2024

Sure, I like this idea. Though instead of inside Diff Options, maybe it'd make more sense to be a "sort" icon in the symbol list itself (or next to each section header?). Mainly, I think it should be obvious when sorting is active. Having the option hidden 2 menus deep could end up being confusing.

For "Reverse function order": that's specifically for objects compiled with -inline deferred, which reverses the function order in the object. To match the order of the functions in the source file, they're (un)reversed in the UI.

Since that's an object-level property (it's set by dtk-template if -inline deferred is set in that object's cflags), and this new feature is a user view preference, it makes sense for them to remain separate. Though how they interact is another question.

@ribbanya
Copy link
Contributor Author

Though how they interact is another question.

I think it'd make sense for the object-level setting to apply first, and the user-level setting has no awareness of it. So the "base order" that the view receives is already reversed if deferred is set.

Where would you want the sorting of the model to happen? I assume it should be persistently sorted so that the render function isn't running the sort every frame.

Is this the line that sorts them currently?

result.sort_by_key(|v| v.address);

@encounter
Copy link
Owner

Yes, that's the baseline sort when loading the object. I think this should happen in the view, though. Probably here, alongside the existing logic:

if section.kind == ObjSectionKind::Code && state.reverse_fn_order {

It's totally fine to do the sort every frame. The beauty of immediate mode UI!

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

No branches or pull requests

2 participants