feat: Add bare-minimum help dialog

[quodlibetor]

Pressing question mark pops up a dialog that tells you you can use the mouse to click menu items to do things and view mouse shortcuts.

Obviously this is not perfect, but it would have saved me a bunch of frustration, and it does a fair amount of the grunt work towards a better help box.

One thing that made me stop here is that the main event loop should probably be split up to do different things depending on if any given dialog is open. Adding the HelpDialog using the current paradigm would have involved approximately tripling the size of the match for no benefit.

I tried to add all the shortcuts to the help dialog, but that (A) crashed because the dialog has a reasonable hard limit on vertical size and I didn't want to investigate scrolling -- a motivated coder could probably add that easily now. And (B) ideally this would maybe use the the existing Menu to build out the shortcuts, but that requires fancier stuff and has actual code design implications that folks would probably like to talk about it and I'm just trying to be a tiny bit helpful.

This is an incremental step towards #25

https://github.com/arxanas/scm-record/assets/277161/43414c61-fb9b-478b-b2ed-9bc9d76de346

[arxanas]

Thanks for implementing!

One thing that made me stop here is that the main event loop should probably be split up to do different things depending on if any given dialog is open. Adding the HelpDialog using the current paradigm would have involved approximately tripling the size of the match for no benefit.

We should probably add some kind of currently-focused-element enum and match on that before routing events further.

Unfortunately, ratatui has no concept of "focus" or "events" (in the bubbling-up GUI way), so we need to build everything ourselves 🙁. The problem will also be especially apparent if we ever add any textarea components, because they'll have to keypresses like q and not let them bubble up.

I used to use cursive, but there were performance problems: https://github.com/arxanas/git-branchless/issues/816. But, at some point in the future, we could try to copy their event-handling design.

but that (A) crashed because the dialog has a reasonable hard limit on vertical size

This is concerning. What if the terminal is too small by nature? Can you add a test with a very small height viewport to make sure that it doesn't crash?

And (B) ideally this would maybe use the the existing Menu to build out the shortcuts

Another simple solution could be to add f-key shortcuts to toggle the menu bar items: https://github.com/arxanas/scm-record/issues/44#issuecomment-2174884566. Those would be easy to add to the button text itself, and naturally expose the entire shortcut interface.

[quodlibetor]

If you let me know which function keys you'd like I'm happy to add them. Maybe F2+, reserving F1 for help?

I think I agree with the idea of a "currently focused" enum and getting rid of the full combinatorial nature of the mega switch. I didn't know how much time I've got to contribute to this so if that's a blocker I can't commit to landing this.

The crash was valid and a programming error -- it was an unsatisfiable constraint because I asked to display a block that was both 90% of my screen tall and required to be less than 20% of my screen. I'm afk, but I bet if you tried to quit when you have a screen that's less than 5 lines tall you'd get the same crash.

[quodlibetor]

Yeah confirmed it exists with the current code:

https://github.com/arxanas/scm-record/assets/277161/e49ad928-cbf6-4148-8dce-78670abe3aa3

It's smaller than I expected, I'll change mine to be one line too to make it exactly as unlikely as the current problem.

Although it is actually worse/more obvious with a narrow terminal:

https://github.com/arxanas/scm-record/assets/277161/2b087157-075f-46c6-9653-0e0302067049

Backtrace on current main d2c754e:

   Compiling scm-record v0.3.0 (/Users/bwm/repos/scm-record/scm-record)
   Compiling scm-diff-editor v0.3.0 (/Users/bwm/repos/scm-record/scm-diff-editor)
    Finished dev [unoptimized + debuginfo] target(s) in 4.85s
     Running `target/debug/scm-diff-editor CHANGELOG.md c2.md`
thread 'main' panicked at scm-record/src/render.rs:218:10:
called `Result::unwrap()` on an `Err` value: UnsatisfiableConstraint
stack backtrace:
   0: rust_begin_unwind
             at /rustc/a28077b28a02b92985b3a3faecf92813155f1ea1/library/std/src/panicking.rs:597:5
   1: core::panicking::panic_fmt
             at /rustc/a28077b28a02b92985b3a3faecf92813155f1ea1/library/core/src/panicking.rs:72:14
   2: core::result::unwrap_failed
             at /rustc/a28077b28a02b92985b3a3faecf92813155f1ea1/library/core/src/result.rs:1652:5
   3: core::result::Result<T,E>::unwrap
             at /rustc/a28077b28a02b92985b3a3faecf92813155f1ea1/library/core/src/result.rs:1077:23
   4: scm_record::render::centered_rect
             at ./scm-record/src/render.rs:209:5
   5: <scm_record::ui::Dialog<Id> as scm_record::render::Component>::draw
             at ./scm-record/src/ui.rs:3331:24
   6: scm_record::render::Viewport<ComponentId>::draw_component
             at ./scm-record/src/render.rs:461:13
   7: <scm_record::ui::QuitDialog as scm_record::render::Component>::draw
             at ./scm-record/src/ui.rs:3264:9
   8: scm_record::render::Viewport<ComponentId>::draw_component
             at ./scm-record/src/render.rs:461:13
   9: <scm_record::ui::AppView as scm_record::render::Component>::draw
             at ./scm-record/src/ui.rs:2260:13
  10: scm_record::render::Viewport<ComponentId>::draw_component
             at ./scm-record/src/render.rs:461:13
  11: <scm_record::render::TopLevelWidget<C> as ratatui::widgets::StatefulWidget>::render
             at ./scm-record/src/render.rs:617:9
  12: ratatui::terminal::frame::Frame::render_stateful_widget
             at /Users/bwm/.cargo/registry/src/index.crates.io-6f17d22bba15001f/ratatui-0.27.0/src/terminal/frame.rs:128:9
  13: scm_record::render::Viewport<ComponentId>::render_top_level
             at ./scm-record/src/render.rs:415:9
  14: scm_record::ui::Recorder::run_inner::{{closure}}
             at ./scm-record/src/ui.rs:552:36
  15: ratatui::terminal::terminal::Terminal<B>::draw
             at /Users/bwm/.cargo/registry/src/index.crates.io-6f17d22bba15001f/ratatui-0.27.0/src/terminal/terminal.rs:269:9
  16: scm_record::ui::Recorder::run_inner
             at ./scm-record/src/ui.rs:551:13
  17: scm_record::ui::Recorder::run_crossterm
             at ./scm-record/src/ui.rs:487:22
  18: scm_record::ui::Recorder::run
             at ./scm-record/src/ui.rs:475:40
  19: scm_diff_editor::main
             at ./scm-diff-editor/src/main.rs:544:11
  20: core::ops::function::FnOnce::call_once
             at /rustc/a28077b28a02b92985b3a3faecf92813155f1ea1/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

[quodlibetor]

If you let me know which function keys you'd like I'm happy to add them.

Actually, looking at the code and it feels like a rabbit hole that I would rather not dive into unless you agree with one of the following:

• I think the right thing to do would be to create a new Event::ExpandMenuItem(TopMenu) with a TopMenu::{File,Edit,Select,View} enum and make the StateUpdate::ClickMenu(idx: u8) -> StateUpdate::ToggleMenu(TopMenu).
• Faster but probably worse: just fake it and create a synthetic Click event on F(n) key presses.
• You you have some other better idea.

With any of these I probably won't get around to it until next weekend, and I can't super commit to doing it, I thought it would be less work to do when I said I'd be happy to.

[quodlibetor]

We should probably add some kind of currently-focused-element enum and match on that before routing events further.

For now the lightest weight thing to do would be to make the top-level match 3 functions instead and do something like:

if self.help_dialog.is_some() {
    self.handle_help_event(event, ...)
} else if self.quit_dialog.is_some() {
    self.handle_quit_event(event, ...)
} else {
    self.handle_base_event(event, ...)
}

but clearly that wouldn't help for when this app has a multi-pane view and I'm not sure if things are intertwined enough currently that that refactoring wouldn't be a pain.

[arxanas]

Backtrace on current main d2c754e:

Oof, that's unfortunate. Thanks for investigating and for the PR!