Skip to content

Conversation

@yaakov-stein
Copy link
Collaborator

@yaakov-stein yaakov-stein commented May 27, 2025

See #1221 for initial issue. This update enables filtering perf events in TUI with fuzzy matching (#1989).

The main challenge was determining how to differentiate standard keybindings from input. To do this, the code checks if the current state is AppState::Event, and if so, places characters into an input buffer as opposed to using the standard character keybindings. The escape button can be used to exit the event selection page (and can now also be used throughout the entire tool as an alternative to 'q'). Here's a quick visual view for how this works:

Screenshot 2025-05-27 at 10 25 56 PM

Additionally, the scrollbar for the event selection menu has been fixed and now functions as expected.

Example usage:

Screen.Recording.2025-05-27.at.6.34.51.PM.mov

@yaakov-stein yaakov-stein marked this pull request as ready for review May 28, 2025 02:29
@JakeHillion
Copy link
Contributor

I think I may have found a bug with this one. Steps to reproduce:

  1. Open scxtop.
  2. Press 'h' to open help.
  3. Press 'e' to search events.
  4. Find an event (fuzzy search works great!).
  5. Press return or escape.

This gets into a pretty weird state where escape/q bounce between help and the search screen, whereas escape or q would be expected to return you to the normal state ordinarily. Not a big deal as you can get out of the help screen by pressing 'd' or something else, but it was pretty confusing.

@yaakov-stein yaakov-stein force-pushed the ystein/enable_event_filtering branch from 8b08db5 to 2f03d5d Compare May 28, 2025 14:51
@yaakov-stein
Copy link
Collaborator Author

Thanks @JakeHillion - nice catch! I was handling the return/escape by going to the previous screen, which is the same way that help handles escape/quit (hence the endless bouncing back and forth if the prev_state is help).

I think the best way to handle this would be to keep the initial prev_state when switching between the help menu and event selection page, so whenever one would enter or return, they'd then go back to whatever "main" screen they were on initially (can be default, LLC, NUMA view etc.). This seems intuitive to me as one who clicks on either help or event selection would want to return to the initial screen they were on after making their selection.

@yaakov-stein yaakov-stein self-assigned this May 28, 2025
let default_style = Style::default().fg(self.theme().text_color());
let chunks = Layout::vertical([Constraint::Min(1), Constraint::Percentage(99)]).split(area);
let chunks = Layout::vertical([
Constraint::Min(1),
Copy link
Contributor

Choose a reason for hiding this comment

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

silly question, is this more than 100%

Copy link
Collaborator Author

@yaakov-stein yaakov-stein May 28, 2025

Choose a reason for hiding this comment

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

Yep, but they shouldn't need to add up to 100 (if they were all Percentages, they would though). Min specifies a minimum amount of space for a given widget and isn't affected by Percentage (so Percentage here can be 99 or 100 as well). The 98 is kind of arbitrary but seems to work nicely when increasing or decreasing the size. Example with another section that has Percentage of 100 (from ratatui docs):

Screenshot 2025-05-28 at 12 13 34 PM

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, thanks for the clarification!

@yaakov-stein yaakov-stein added this pull request to the merge queue May 28, 2025
Merged via the queue into sched-ext:main with commit 8cb0d4b May 28, 2025
16 checks passed
@yaakov-stein yaakov-stein deleted the ystein/enable_event_filtering branch May 28, 2025 19:27
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.

3 participants