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

objdiff-gui: Implement keyboard shortcuts #139

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

LagoLunatic
Copy link

Keyboard-only navigation demonstration:

objdiff-gui-hotkeys.mp4

This PR implements the following keyboard shortcuts:

Symbol list view:

  • Enter or Space or Mouse5: Enter the diff view for the symbol that is currently highlighted.
  • Up/Down or W/S: Select the symbol above/below the currently selected symbol. If no symbol is highlighted, select the topmost symbol.
  • PageUp/PageDown: Scroll up/down by the window height.
  • Home/End: Scroll to the top/bottom.
  • Ctrl+F: Give focus to the object filter input. (Note: egui's builtin Tab/Shift+Tab/Enter shortcuts already work fine for selecting an object from the filtered list, so I didn't need to add new ones for that.)
  • Ctrl+S: Give focus to the symbol filter input.

Function/data/extab diff view:

  • Backspace or Escape or Mouse4: Go back to the symbol list.
  • Up/Down or W/S: Scroll up/down by 10% of the window height.
  • PageUp/PageDown: Scroll up/down by the window height.
  • Home/End: Scroll to the top/bottom.

Function diff view only:

  • Ctrl+T: Change target function mapped to this base function.
  • Ctrl+B: Change base function mapped to this target function.

I also wanted to add a shortcut to open the context menus via the keyboard, but I don't think it's possible in egui :/

Most of these are pretty self-explanatory, but the shortcuts to select the symbol above/below the current one in the listing required a few changes to how objdiff handles the highlighted symbols.
The short version is that the highlighted symbols pair is now treated more like a permanent cursor that you can use to navigate around without the mouse.
Long version:

  • The highlighted symbol is no longer cleared when backing out of a diff view. This is to make it easier to re-enter the same symbol you were just diffing, or to go to the one above it, etc.
  • When the highlighted symbol is changed via the keyboard, the newly selected symbols will both be scrolled into view one time. When using the mouse, this is not done.
  • Using the mouse to highlight a symbol that has no counterpart on the opposite side now gives it a blue (permanent) highlight like other symbols, instead of just a grey (temporary) highlight. This is so you can start by selecting a symbol with the mouse, and then move the cursor around with the keyboard to continue selecting other symbols.

Also, the "split view" where half the window is the function view and the other half is the symbol list view is supported. The symbol list's shortcuts work normally here, while the function diff half has the up/down shortcuts disabled so it doesn't scroll accidentally while navigating the symbol list, but the PageUp/PageDown/Home/End shortcuts are still enabled in case you do want to scroll the function diff in the split view.

Let me know if anything could be improved!

e.g. The symbol list was stealing the W/S key presses when typing into the symbol filter text edit.

If the user actually wants to use these shortcuts while a widget is focused, they can simply press the escape key to unfocus all widgets and then press the shortcut.
This is for consistency with egui's builtint enter/space hotkey for interacting with the focused widget.
The flag is cleared after one scroll to avoid doing it continuously, but this breaks when we need to scroll to both the left and the right symbol at the same time. So now each side has its own flag to keep track of this state independently.
DiffViewState::post_update is where the flag gets set, so clearing it right before that at the start of the function seems to make the most sense, instead of doing it in App::update.
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.

1 participant