Skip to content

fix(git): prevent panic when opening git menu with no selected revision#571

Merged
idursun merged 1 commit intoidursun:mainfrom
baggiiiie:yc/fix/git-panic
Mar 2, 2026
Merged

fix(git): prevent panic when opening git menu with no selected revision#571
idursun merged 1 commit intoidursun:mainfrom
baggiiiie:yc/fix/git-panic

Conversation

@baggiiiie
Copy link
Copy Markdown
Collaborator

Root cause:

  • revisions.SelectedRevisions() can call
    jj.NewSelectedRevisions(m.SelectedRevision()) when no row is selected.
  • m.SelectedRevision() can be nil.
  • SelectedRevisions previously kept nil entries, so
    git.NewModel/createMenuItems dereferenced commit.GetChangeId() on
    a nil *jj.Commit and panicked.
  • when the jj repo is big, initial load/refresh might not have produced
    rows before revision.SelectedRevision() is called.

Fixes:

  • Filter nil *Commit values in jj.NewSelectedRevisions before storing
    them.
  • Keep regression coverage for git model construction with a nil
    selected revision (no panic).
  • Add a constructor unit test proving nil revisions are dropped.
  • createMenuItems only iterates non-nil commits after construction
    filtering.
  • this removes the invalid dereference path while preserving existing
    behavior for valid selections.

Closes #570

Root cause:
- `revisions.SelectedRevisions()` can call
  `jj.NewSelectedRevisions(m.SelectedRevision())` when no row is selected.
- `m.SelectedRevision()` can be nil.
- `SelectedRevisions` previously kept nil entries, so
  `git.NewModel/createMenuItems` dereferenced `commit.GetChangeId()` on
  a nil *jj.Commit and panicked.
- when the jj repo is big, initial load/refresh might not have produced
  rows before `revision.SelectedRevision()` is called.

Fixes:
- Filter nil *Commit values in `jj.NewSelectedRevisions` before storing
  them.
- Keep regression coverage for git model construction with a nil
  selected revision (no panic).
- Add a constructor unit test proving nil revisions are dropped.
- createMenuItems only iterates non-nil commits after construction
  filtering.
- this removes the invalid dereference path while preserving existing
  behavior for valid selections.

Closes idursun#570
@baggiiiie baggiiiie requested a review from idursun as a code owner March 2, 2026 05:33
Copy link
Copy Markdown
Owner

@idursun idursun left a comment

Choose a reason for hiding this comment

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

Hey thanks! That's exactly what I suspected. I assume we have the same problem in bookmarks too, if so we can fix it later.

@idursun idursun merged commit 1368a25 into idursun:main Mar 2, 2026
4 checks passed
baggiiiie added a commit to baggiiiie/jjui that referenced this pull request Mar 2, 2026
SelectedRevision() can return nil when the cursor is out of bounds
(e.g., during initial load of a large repo). This nil was passed
directly to bookmarks.NewModel, causing a nil pointer dereference
at bookmarks.go:563.

Add an early return when SelectedRevision() is nil, consistent with
the existing nil-filtering fix in NewSelectedRevisions for the git
module 

related to idursun#570 and idursun#571
@baggiiiie
Copy link
Copy Markdown
Collaborator Author

@idursun good call, let me send another fix

idursun pushed a commit that referenced this pull request Mar 2, 2026
SelectedRevision() can return nil when the cursor is out of bounds
(e.g., during initial load of a large repo). This nil was passed
directly to bookmarks.NewModel, causing a nil pointer dereference
at bookmarks.go:563.

Add an early return when SelectedRevision() is nil, consistent with
the existing nil-filtering fix in NewSelectedRevisions for the git
module 

related to #570 and #571
tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Mar 11, 2026
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [idursun/jjui](https://github.com/idursun/jjui) | minor | `v0.9.12` → `v0.10.0` |

MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).

**Proposed changes to behavior should be submitted there as MRs.**

---

### Release Notes

<details>
<summary>idursun/jjui (idursun/jjui)</summary>

### [`v0.10.0`](https://github.com/idursun/jjui/releases/tag/v0.10.0)

[Compare Source](idursun/jjui@v0.9.12...v0.10.0)

Wow, a major release after a very long time. As promised, `v0.10` is ready, and it comes with **breaking changes**.

First, thank you to everybody who contributed code, bug reports, ideas, and testing for this release. In particular, thanks to [@&#8203;baggiiiie](https://github.com/baggiiiie) for various features and fixes, [@&#8203;nickchomey](https://github.com/nickchomey) for contributions and continuous feedback, and [@&#8203;vic](https://github.com/vic) for updating the documentation website. Thanks as well to everyone else for various code contributions, reporting issues, and verifying fixes.

We changed a lot in `v0.10`, but the biggest shift is that we finally got rid of some **legacy configuration** and moved to a unified **actions + bindings** model. Old concepts like `[custom_commands]` and `[leader]` are now replaced by a more consistent system built around **actions**, **bindings**, and first-class support for leader-style key sequences.

This release also introduces `config.lua`, which makes it much easier to customise and extend `jjui` with **real scripting** instead of only static configuration. Between `config.toml`, `config.lua`, Lua actions, and bindings, much more of the UI can now be customised in one consistent way.

The documentation has been updated with [**migration notes**](https://idursun.github.io/jjui/migrating/v0_10/) and examples such as the [**Lua Cookbook**](https://idursun.github.io/jjui/lua-cookbook/).

From my testing it looks ready, but I am sure more rough edges will show up once more people start using it, so please keep reporting issues as you find them.

#### ⚠️ Breaking Changes

`v0.10` replaces the **legacy keybinding and customisation model** with the new **actions + bindings** system.

- `[keys]` configuration is no longer used.
- `[custom_commands]` is replaced by `[[actions]]` and `[[bindings]]`.
- `[leader]` sequences are replaced by sequence bindings via `seq`.

If you have an existing configuration, plan to **migrate it** rather than expecting a drop-in upgrade from `v0.9.x`.

#### ✨Highlights

- **Unified actions + bindings system.** Keyboard input now flows through a single dispatch pipeline, default bindings are declared in TOML, actions are discoverable, and custom behaviour can be defined in both TOML and Lua.
- **Much stronger scripting support.** Lua `setup` now receives the full config plus runtime context, generated intent-backed actions replace handwritten drift-prone bridges, and new helpers like `wait_close()` and `wait_refresh()` make multi-step scripts more predictable.
- **Upgraded TUI foundations.** Moving from **Bubble Tea v1** to **v2** brings better terminal support, a new rendering engine for better performance, and better input handling, including support for bindings like `ctrl+space` and dedicated `shift+...` style keys.
- **Improved diff and selection workflows.** The `diff` view now supports word wrap, `revisions` and `details` support modifier-based mouse selection, and several `details` and `evolog` interactions are more consistent.

#### 🚀 New and Improved

##### Command History

- Added [**command history**](https://idursun.github.io/jjui/command-history/), available with `shift+w`, to review dismissed command runs together with their output.
- Moved **running command** information out of the status area and into dedicated `flash` messages, so the command and its output are now shown together in one place.

##### Help and Status

- Reintroduced a dedicated **Help view** for full keybinding discovery, with filtering and scrolling, alongside the existing inline help.
- Changed **status/help** binding presentation to **column-major order** so related bindings are easier to scan.

##### Lua and Configuration

- Shipped the unified **actions + bindings** architecture introduced in `v0.10`.
- Exposed the full config object to Lua `setup`, including runtime context such as repository path via `config.repo`, and terminal state via `config.terminal.dark_mode`, `config.terminal.fg`, and `config.terminal.bg`.
- Standardised closeable action naming around `open_*` and aligned apply/close semantics for more predictable scripted workflows.

##### Oplog

- Added **quick search** to `oplog` and shared the search behavior with revision search.

##### Evolog

- Changed `evolog` preview defaults to show **interdiff** output, matching `jj evolog -p` more closely.

##### Abandon

- Added `s` in `abandon` mode to target descendants of the selected revision(s).

##### Revset

- Kept `revset` editing open when the expression is invalid, improving iteration while editing.
- Improved `revset` editing so function completion inserts `(` when appropriate, while the function list display is cleaner and less noisy.

##### Diff

- Added **word wrap** in the `diff` view, with explicit wrapped and unwrapped modes plus soft-wrap-aware scrolling.
- Added `g` and `G` in the `diff` view to jump to the top and bottom.

##### Details and Revisions

- Added `Ctrl+click` single-item toggle and `Alt+click` range selection in `revisions` and `details`.
- Added a `revisions.details.select_file` action for finer control in `details` workflows.

#### 🛠️ Fixes

- Fixed rendering issues with **double-width characters**.
- Simplified divergent revision handling by using `change_id/offset` directly from the `jj log` prefix template.
- Fixed `git push --all` behaviour for deleted bookmarks and corrected related command descriptions.

#### ⚙️ Internal Updates

- Upgraded to **Bubble Tea v2**.
- Replaced remaining `cellbuf` usage with `ultraviolet`.
- Hardcoded model-level key handling has been removed in favor of dispatcher-driven intents.
- Continued internal cleanup around rendering, overlays, display context, and generated action plumbing to support the new architecture.

#### What's Changed

- Unified Actions + Bindings system by [@&#8203;idursun](https://github.com/idursun) in [#&#8203;533](idursun/jjui#533)
- feat(oplog): add QuickSearch to oplog, share logic with revision search by [@&#8203;baggiiiie](https://github.com/baggiiiie) in [#&#8203;521](idursun/jjui#521)
- fix: add missing keys in bindings by [@&#8203;baggiiiie](https://github.com/baggiiiie) in [#&#8203;538](idursun/jjui#538)
- feat(evolog): change default evolog command to show interdiff by [@&#8203;academician](https://github.com/academician) in [#&#8203;539](idursun/jjui#539)
- fix: add back `esc` to clear checked revisions by [@&#8203;baggiiiie](https://github.com/baggiiiie) in [#&#8203;541](idursun/jjui#541)
- remove gh-pages workflow from main branch. by [@&#8203;vic](https://github.com/vic) in [#&#8203;542](idursun/jjui#542)
- docs: fix readme links by [@&#8203;baggiiiie](https://github.com/baggiiiie) in [#&#8203;549](idursun/jjui#549)
- Update CONTRIBUTING.md for documentation by [@&#8203;vic](https://github.com/vic) in [#&#8203;547](idursun/jjui#547)
- feat(lua): add "filter" and "ordered" option to `choose` lua api by [@&#8203;baggiiiie](https://github.com/baggiiiie) in [#&#8203;548](idursun/jjui#548)
- feat(abandon): use `s` for selecting descendants by [@&#8203;idursun](https://github.com/idursun) in [#&#8203;544](idursun/jjui#544)
- feat: add change\_offset in jj log prefix template for divergent revisions by [@&#8203;baggiiiie](https://github.com/baggiiiie) in [#&#8203;550](idursun/jjui#550)
- fix: make bookmark input visible when user has custom template-aliases by [@&#8203;baggiiiie](https://github.com/baggiiiie) in [#&#8203;551](idursun/jjui#551)
- fix(evolog): keep restore mode navigation on revisions and esc close by [@&#8203;baggiiiie](https://github.com/baggiiiie) in [#&#8203;554](idursun/jjui#554)
- Expose full config to Lua setup and add runtime context (repo + terminal) by [@&#8203;idursun](https://github.com/idursun) in [#&#8203;553](idursun/jjui#553)
- feat(flash): command history by [@&#8203;idursun](https://github.com/idursun) in [#&#8203;556](idursun/jjui#556)
- Update to bubbletea v2 by [@&#8203;idursun](https://github.com/idursun) in [#&#8203;558](idursun/jjui#558)
- Ctrl+click and Alt+click for single and range selection by [@&#8203;idursun](https://github.com/idursun) in [#&#8203;563](idursun/jjui#563)
- feat: add `(` to functions when autocompleted by [@&#8203;nickchomey](https://github.com/nickchomey) in [#&#8203;564](idursun/jjui#564)
- feat(status): change display to column major order by [@&#8203;baggiiiie](https://github.com/baggiiiie) in [#&#8203;569](idursun/jjui#569)
- fix: remove parentheses from functions list, but add when autocomplete by [@&#8203;nickchomey](https://github.com/nickchomey) in [#&#8203;567](idursun/jjui#567)
- fix(git): prevent panic when opening git menu with no selected revision by [@&#8203;baggiiiie](https://github.com/baggiiiie) in [#&#8203;571](idursun/jjui#571)
- Refactor/Lua bridge by [@&#8203;idursun](https://github.com/idursun) in [#&#8203;572](idursun/jjui#572)
- fix(ui): guard against nil commit in OpenBookmarks intent by [@&#8203;baggiiiie](https://github.com/baggiiiie) in [#&#8203;573](idursun/jjui#573)
- feat(diff): word wrap by [@&#8203;idursun](https://github.com/idursun) in [#&#8203;575](idursun/jjui#575)
- Add pgup/pgdown keybindings to evolog and details by [@&#8203;academician](https://github.com/academician) in [#&#8203;576](idursun/jjui#576)
- feat(lua): make default actions overridable and expose jjui.builtin.\* by [@&#8203;idursun](https://github.com/idursun) in [#&#8203;577](idursun/jjui#577)
- feat(help): add "help view" bound to `f1` by [@&#8203;idursun](https://github.com/idursun) in [#&#8203;578](idursun/jjui#578)

#### New Contributors

- [@&#8203;academician](https://github.com/academician) made their first contribution in [#&#8203;539](idursun/jjui#539)

**Full Changelog**: <idursun/jjui@v0.9.12...v0.10.0>

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My41OS40IiwidXBkYXRlZEluVmVyIjoiNDMuNTkuNCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiUmVub3ZhdGUgQm90IiwiYXV0b21hdGlvbjpib3QtYXV0aG9yZWQiLCJkZXBlbmRlbmN5LXR5cGU6Om1pbm9yIl19-->
@baggiiiie baggiiiie deleted the yc/fix/git-panic branch March 12, 2026 17:02
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.

nil pointer derefence

2 participants