#536: [retry] Migrate activity-bar keyboard nav onto quadraui AppShell (retire hand-rolled toolbar_selected arithmetic) — consume quadraui#386 - #681
Merged
JDonaghy merged 2 commits intoAug 27, 2026
Conversation
…ursor Retires vimcode's hand-rolled activity-bar nav arithmetic in favour of quadraui#386's `AppShell` keyboard cursor. Behaviour-preserving. quadraui#386's cursor **does** handle a variable panel count — the open scope question in the issue. `AppShell::add_panel`/`remove_panel` adjust `activity_cursor` alongside `active_panel`, and the cursor spans `panels` then `bottom_items` as one saturating sequence. That is exactly vimcode's painted order (hamburger, the six fixed panels, the dynamic extension panels sorted by name, then bottom-pinned Settings), so the migration covers the dynamic ext-panel range too — no static-only carve-out was needed. What changed (`core/engine/sidebar.rs`): * `activity_bar_move_down`/`activity_bar_move_up` are now two-line wrappers over `activity_bar_step`, which builds `activity_nav_shell()` — an `AppShell` whose panel/bottom-item lists mirror the TUI's live `ShellConfig` — and delegates to `activity_select_next()` / `activity_select_prev()`. The bespoke `0→1…→6→8…→max_ext→7(settings)` chain is gone. * `activity_bar_activate` dispatches on the item **id** rather than re-deriving `sel - 8`, so the `8 + idx` extension arm is gone too. * `activity_bar_toolbar_idx_for_active_panel`'s hardcoded panel→index table is now a `FIXED_ACTIVITY_PANEL_IDS` lookup. * The legacy `activity_bar_selected: u16` index space is kept (≈15 call sites across both backends and the plugin layer depend on it), but the index↔id translation now lives in exactly one pair of functions — `activity_bar_item_id` / `activity_bar_idx_for_item_id` — backed by two named constants instead of literals scattered across three files. * Bonus fix: a selection left pointing at an extension panel that `:PluginReload` dropped used to wedge `j` (no arm matched); it now clamps to the last item first, mirroring `AppShell::remove_panel`. `render.rs`: `kbd_sel` matches by panel id via `Engine::activity_bar_selected_item_id()` instead of comparing numeric toolbar indices, so `build_activity_bar`'s `fixed` array loses its `u16` column and the ext loop loses its `8 + i`. `tui_main/shell_app.rs`: `HAMBURGER_PANEL_ID` promoted to `core::engine::sidebar` so the core can name keyboard slot 0 — one definition instead of a literal per call site. Tests: five new black-box tests in `tui_main/panels.rs` that probe the **rasterised** activity-bar strip (which row carries the selection background) rather than reading `activity_bar_selected`, covering the fixed-panel walk, bottom-pinned Settings, saturation at both ends, the ext-panel splice between AI and Settings in both directions, and stale ext-index recovery. Verified non-vacuous by mutation (clamping the cursor to 6 fails 3 of the 5). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`git_api_stash_list_returns_table` failed the Test stage with
`assertion failed: found`, while the rest of the suite was green and the
test passed in isolation. It is a harness race, not a regression from the
activity-bar work on this branch.
`plugin_with` in tests/extensions.rs built its scratch plugin directory
from `code.len()` ("simple discriminator"), which does not discriminate:
any two scripts of the same byte length share one path. Exactly one pair
collides today — `git_api_stash_list_returns_table` and
`git_api_blame_file_returns_table`, both 174 bytes, both resolving to
/tmp/vc_git_api_test_174. The helper opens with `remove_dir_all(&dir)`,
so under cargo's parallel test threads one test can delete the other's
test.lua in the window before `load_plugins_dir` reads it.
`PluginManager::load_plugins_dir` bails silently when `read_dir` fails
(src/core/plugin.rs:304), so no command is registered and the victim's
`assert!(found)` panics.
Key the directory on pid + a monotonic counter instead, so every call
site gets its own path and no two tests can contend. Also remove the dir
after `load_plugins_dir` returns — it loads eagerly, so the files are
dead weight, and the old helper leaked one dir per distinct script
length into /tmp.
Tests: `plugin_with_uses_a_distinct_dir_for_same_length_scripts` asserts
two same-length scripts get different dirs and that both plugins load.
Verified it goes red against the unfixed helper: reinstating the
`code.len()` keying fails it deterministically with
`left: "/tmp/vc_git_api_test_64", right: "/tmp/vc_git_api_test_64"`.
`plugin_scratch_dirs_are_cleaned_up` covers the leak.
Test-harness change only — no user-visible behaviour touched, so the
black-box driver rule does not apply.
cargo build / cargo fmt --check / cargo clippy -- -D warnings /
cargo clippy --no-default-features -- -D warnings / cargo test all
clean (2300 lib + 46 binaries, 0 failed). Ran the extensions binary 10x
to confirm the flake is gone.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
JDonaghy
force-pushed
the
issue-536-migrate-activity-bar-keyboard-nav-onto-q
branch
from
August 27, 2026 18:05
3b1bfdc to
9918ecb
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #536
Automated merge from the coordinator for assignment 7ef4f429add8 on issue #536.
Worker branch:
issue-536-migrate-activity-bar-keyboard-nav-onto-q→develop.