oplog,list: refactor scrolling with Scrollable/StreamableList interface#429
Merged
idursun merged 1 commit intoidursun:mainfrom Dec 20, 2025
Merged
oplog,list: refactor scrolling with Scrollable/StreamableList interface#429idursun merged 1 commit intoidursun:mainfrom
idursun merged 1 commit intoidursun:mainfrom
Conversation
feacba7 to
037e5ad
Compare
Contributor
|
I can confirm that this fixes #360 with the following config: 👍 [keys]
scroll_down = []
scroll_up = []
[keys.preview]
half_page_down = ["pgdown"]
half_page_up = ["pgup"] |
idursun
requested changes
Dec 17, 2025
internal/ui/common/list/list.go
Outdated
| } | ||
|
|
||
| // IListScroll defines the interface for list models that support navigation | ||
| type IListScroll interface { |
Owner
There was a problem hiding this comment.
I am not sure about this interface, more specifically the shape of it. At first glance, it has IList and IListCursor which means this is a list and it will have a cursor. But then there's navigationconfig, which has ContextName, NavigationMessage, some concept of streaming all munched together. If I were to implement something that implements this interface, then I would have to look at its implementations first to make sense what all those mean. (Obviously I know what they mean, but I am saying all these things from the point of someone who is interacting with the codebase the very first time).
I would probably call it IScrollableList which conveys the message that this is a list, and it is scrollable. And then I would add methods that would make sense for a scrollable list (e.g. VisibleRange() (int, int)
I would probably move the streaming related methods to a new interface (e.g. IStreamableList).
These are just ideas that I came up with in a few mins, the actual implementation can be different but probably will be along those lines.
Collaborator
Author
There was a problem hiding this comment.
thanks for the feedback!
yup makes sense! the previous interface wasn't clear and a NavigationConfig is quite a vague concept to capture all information needed for scrolling.
i have updated IScrollableList and IStreamableList, much cleaner now
internal/ui/common/list/list_test.go
Outdated
| if result.NewCursor != 0 { | ||
| t.Errorf("Expected cursor to remain 0 for empty list, got %d", result.NewCursor) | ||
| } | ||
| if result.NavigateMessage != nil { |
Owner
There was a problem hiding this comment.
You should be able to use assert.NotNil here.
90ff104 to
f902096
Compare
Refactored revisions/operation_log scrolling to have an IScrollableList interface, with `scroll` method for navigation, which handles: - Boundary detection (top/bottom of list) - Page vs. single-step navigation - Streaming/infinite scroll support - User feedback messages (reaching top/bottom of list) Also added a IStreamableList to handle streaming data that needs to by loaded dynamically
f902096 to
9a6d31c
Compare
tmeijn
pushed a commit
to tmeijn/dotfiles
that referenced
this pull request
Jan 9, 2026
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [idursun/jjui](https://github.com/idursun/jjui) | patch | `v0.9.8` → `v0.9.9` | 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.9.9`](https://github.com/idursun/jjui/releases/tag/v0.9.9) [Compare Source](idursun/jjui@v0.9.8...v0.9.9) ### Release Notes Another release with small improvements and bug fixes. Thanks to all contributors! #### 🎉 New Features ##### Custom Commands & Lua API Enhancements - **Custom Commands with Sequence Keys** ([#​420](idursun/jjui#420)) - Added `key_sequence` property allowing custom commands to be invoked with multiple key presses in sequence - Added `desc` property for command descriptions - Introduced sequence overlay UI showing available key sequences when first key is pressed - Example: `key_sequence = ["w", "b", "l"]` - **Lua API: Choose Method and UI** ([#​427](idursun/jjui#427)) ([#​442](idursun/jjui#442)) - New `choose()` function for interactive selection prompts in Lua scripts - New `input()` function to prompt users for text input with customizable title and prompt - New `split_lines()` function for text processing - **Lua API: Await on Operation Results** ([#​422](idursun/jjui#422)) - `start_inline_describe()` now returns boolean indicating if operation was applied or cancelled - Enables conditional command execution based on user actions - Fixes [#​310](idursun/jjui#310) - **Lua API: Interactive Commands** ([commit 8b257263](idursun/jjui@8b257263)) - Added `jj_interactive` Lua function for interactive jj command execution ##### Navigation & UI Improvements - **Ace Jump for Operations** ([#​445](idursun/jjui#445)) - Pressing 'f' in set\_parents/duplicate/rebase/squash modes now triggers ace jump - After jump completes, returns to the original operation mode instead of normal mode - Closes [#​394](idursun/jjui#394) - **Preview Width Variable** ([#​452](idursun/jjui#452)) - Added `$preview_width` placeholder variable for preview commands - Exposes actual view width (in columns) to enable tools like delta to use `--side-by-side` correctly - Width updates dynamically when preview pane is resized - Similar to fzf's `$FZF_PREVIEW_COLUMNS` - **Configurable Flash Message Display Time** ([#​456](idursun/jjui#456)) - New config key: `ui.flash_message_display_seconds` (default: 4) - Special value `0` means messages display until manually dismissed - Fixes [#​455](idursun/jjui#455) - **Page Up/Down Key Configuration** ([#​437](idursun/jjui#437)) - ScrollUp/Down keys now registered in config instead of hardcoded - Keys exposed to configuration for customization - Fixes [#​360](idursun/jjui#360) ##### SSH & Authentication - **SSH Askpass Support** ([#​423](idursun/jjui#423)) - New `[ssh] hijack_askpass` setting to prompt for SSH passphrases/PINs within jjui - Works on Linux and macOS - Properly handles prompt overriding and cancellation - Fixes [#​100](idursun/jjui#100) #### 🐛 Bug Fixes - **Exec Command History** ([#​458](idursun/jjui#458)) - Fixed issue where selected command history wasn't applied in exec mode - Input value now properly updated when selecting from fuzzy/regex suggestions - Selected commands correctly saved to history - **Menu Pagination Display** ([#​446](idursun/jjui#446)) - Fixed incorrect `%d/%d` pagination display - Height now calculated before pagination render - Added tab/shift+tab to short help menu - Fixes [#​444](idursun/jjui#444) - **Flash Message Width** ([#​432](idursun/jjui#432)) - Added maxWidth (50% of screen) to flash message rendering - Messages now properly line-wrap instead of extending beyond window width - **Operation Log Refresh** ([#​431](idursun/jjui#431)) - Operation log now returns Refresh and SelectionChanged messages upon closing - Fixes [#​430](idursun/jjui#430) - **Custom Commands List Sorting** ([commit 3fa9783a](idursun/jjui@3fa9783a)) - Fixed custom commands list to use stable sort - Fixes [#​424](idursun/jjui#424) - **JJ Error Pass-through** ([#​421](idursun/jjui#421)) - jjui now properly passes through stderr from jj commands - Error messages are more informative and show actual jj errors - **Navigation Message Display** ([commit 94a4a874](idursun/jjui@94a4a874)) - Navigation messages now only shown for paged scrolls #### What's Changed - main: pass through jj error by [@​baggiiiie](https://github.com/baggiiiie) in [#​421](idursun/jjui#421) - feat(lua): add ability to await on operation results (cancelled/applied) by [@​idursun](https://github.com/idursun) in [#​422](idursun/jjui#422) - oplog: return Refresh and SelectionChanged upon oplog closing by [@​baggiiiie](https://github.com/baggiiiie) in [#​431](idursun/jjui#431) - feat(nix): add comprehensive nix flake by [@​doprz](https://github.com/doprz) in [#​426](idursun/jjui#426) - Add option to hijack SSH Askpass to prompt for passphrase/pin by [@​oliverpool](https://github.com/oliverpool) in [#​423](idursun/jjui#423) - feat(lua): add choose method and ui by [@​idursun](https://github.com/idursun) in [#​427](idursun/jjui#427) - flash: add maxWidth to flash msg rendering by [@​baggiiiie](https://github.com/baggiiiie) in [#​432](idursun/jjui#432) - keys: add pageup/down to keys config by [@​baggiiiie](https://github.com/baggiiiie) in [#​437](idursun/jjui#437) - chore(github): Add Adda0 as an automatic reviewer for Nix-related changes by [@​Adda0](https://github.com/Adda0) in [#​440](idursun/jjui#440) - feat: add .editorconfig by [@​doprz](https://github.com/doprz) in [#​434](idursun/jjui#434) - Add input and log to custom\_commands API by [@​ArnaudBger](https://github.com/ArnaudBger) in [#​442](idursun/jjui#442) - oplog,list: refactor scrolling with Scrollable/StreamableList interface by [@​baggiiiie](https://github.com/baggiiiie) in [#​429](idursun/jjui#429) - menu: calculate height before pagination render by [@​baggiiiie](https://github.com/baggiiiie) in [#​446](idursun/jjui#446) - operations: enable ace jump for set\_parents/duplicate/rebase/squash by [@​baggiiiie](https://github.com/baggiiiie) in [#​445](idursun/jjui#445) - feat: make flash message display time configurable by [@​living180](https://github.com/living180) in [#​456](idursun/jjui#456) - feat: add $width variable for preview commands by [@​pablospe](https://github.com/pablospe) in [#​452](idursun/jjui#452) - status: fix exec command history not applied by [@​baggiiiie](https://github.com/baggiiiie) in [#​458](idursun/jjui#458) #### New Contributors - [@​doprz](https://github.com/doprz) made their first contribution in [#​426](idursun/jjui#426) - [@​oliverpool](https://github.com/oliverpool) made their first contribution in [#​423](idursun/jjui#423) - [@​living180](https://github.com/living180) made their first contribution in [#​456](idursun/jjui#456) - [@​pablospe](https://github.com/pablospe) made their first contribution in [#​452](idursun/jjui#452) **Full Changelog**: <idursun/jjui@v0.9.8...v0.9.9> </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **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:eyJjcmVhdGVkSW5WZXIiOiI0Mi43NS4xIiwidXBkYXRlZEluVmVyIjoiNDIuNzUuMSIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiUmVub3ZhdGUgQm90IiwiYXV0b21hdGlvbjpib3QtYXV0aG9yZWQiLCJkZXBlbmRlbmN5LXR5cGU6OnBhdGNoIl19-->
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
3 participants
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.
oplog,list: refactor scrolling with Scrollable/StreamableList interface
Refactored revisions/operation_log scrolling to have an IScrollableList
interface, with
scrollmethod for navigation, which handles:Also added a IStreamableList to handle streaming data that needs to by
loaded dynamically