-
Notifications
You must be signed in to change notification settings - Fork 32
Tag & Ref Management, UX Improvements #112
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
- Create tag dialog: name input, optional message, push to origin checkbox - Delete tag dialog: select from list, delete from remote option - Tags sorted with semver awareness - UI updates immediately after tag operations
- Add guards for empty list/height in navigation methods (select_next,
select_last, select_low, select_middle, scroll_up, scroll_down_height,
select_parent, select_next_match, select_prev_match)
- Fix search navigation when filter is active: search_matches uses real
indices but total was filtered count, causing index out of bounds
- Add is_index_visible() and select_real_index() helpers for proper
index conversion between filtered and full commit lists
- Track last_search_ignore_case/fuzzy to invalidate incremental search
cache when settings change
- Show relevant hotkeys based on current view (list, detail, refs, etc.) - Change ignore_case_toggle keybind from Ctrl-g to Alt-c (industry standard)
- App owns Repository instead of borrowing (enables mutation) - Add add_ref/remove_ref methods to Repository for ref tracking - take_list_state() now returns Option, views handle gracefully - event.rs: channel errors handled without panic - ref_list: show error message on tree build failure - ref_list: fix O(n²) performance in refs_to_ref_tree_nodes
|
Thank you for your interest in this project and for taking the time to put together this PR. As you mentioned, I’ve also thought that having branch and tag operations in this application could make sense, so at a high level I agree that the idea itself is reasonable. That said, for the reasons below, I’m hesitant to accept this PR in its current form. UI concerns This PR introduces dialog-based UIs for each operation. I understand that some kind of UI is necessary for branch and tag management, but I think this needs more careful consideration. One of the goals of this application is to avoid becoming a full-featured or complex TUI git client. For that reason, I’ve been intentionally avoiding “TUI-style” interactions so far. Scope of changes This PR also includes a wide range of changes, such as reload functionality, filtering features, performance optimizations, and changes to the default key bindings. While I understand that some of these (for example, reload support) may be necessary to implement the proposed features, I don’t think they should all be introduced in a single PR. I also think filtering should be discussed and designed after we’ve clarified how it affects graph rendering. Elements like the hints displayed at the bottom of the screen relate to both UI and feature scope, and their current behavior is also a deliberate choice to avoid a more TUI-heavy interface (though I’m not claiming this is necessarily the best approach). For these reasons, I’m not comfortable merging this PR as-is. I appreciate the effort and ideas, and I think it would be better to discuss the design and scope in smaller, more focused steps. |
- Add graph_color_set and cell_width_type fields to App struct - Reload Repository from disk on refresh - Recalculate Graph and rebuild GraphImageManager - Rebuild CommitListState with updated commits and refs - Show success/error notification after refresh
…me input Use char-based slicing instead of byte-based slicing when truncating long input values for display.
- Update UI when local deletion succeeds but remote fails - Use safe bounds checking with .get() in delete_tag
|
Thank you for the feedback. I'm glad we agree that branch and tag operations would be a valuable addition to the project — that's a solid foundation to build on. These changes are deliberate, well-considered, and aligned with what has become standard practice in successful applications in this space. UI Concerns Regarding the UI concerns: to be honest, I'm not sure what alternative approach you have in mind. Branch and tag operations inherently require some form of interactive UI — I don't see how to implement them otherwise. The current implementation follows ui patterns established by the most popular tools in this space, particularly Git Graph for VS Code, which has become something of an industry standard with millions of users. Clarification Needed I'm also not entirely clear on what "TUI-heavy interface" means in this context. If you have a specific vision for how these interactions should work, I'd genuinely like to understand it. I'm open to iterating on the implementation, but I'd need something concrete to work towards. Next Steps I've put together a detailed list of all the changes below. Could you:
That way we can reduce the scope and have a focused discussion on specific points. I'm also happy to split this into smaller PRs if that makes the review easier. Let me know how you'd like to proceed. Tag Management - Create Tag DialogFiles:
Tag Management - Delete Tag DialogFiles:
Delete Refs from Refs PanelFiles:
Pending Overlay ComponentFiles:
Context-Aware Hotkey Hints in FooterFiles:
Keybind ChangesFiles:
Search/Filter Index Conflict FixFiles:
Error Handling & Panic RemovalFiles:
Performance Fix - O(n²) → O(n)Files:
Refresh Button (Full Implementation)Files:
Rc RefactoringFiles:
UX ImprovementsFiles:
Test ScriptFiles:
|
Hey Kyosuke, Hope you're doing well! Here's what this PR brings to Serie:
Motivation
Serie is a terminal-native git history viewer: instant startup, visual branch topology, keyboard-driven navigation. It fills the gap between git log and full git clients like lazygit - a focused tool for browsing commit history, similar to VS Code Git Graph but available everywhere.
My personal workflow involves frequent releases through CI/CD, which means constant tag management. Serie excels at navigating history and finding the right commit, but creating a tag still requires a context switch: copy the hash, open terminal, run git tag, then git push --tags. Even lazygit doesn't solve this completely - it can create tags but won't push them to origin.
This PR adds complete ref management to Serie: create/delete tags and branches, push to remote - all without leaving the interface. Tags and branches are a natural fit for a history viewer. They don't modify history, they're reversible; "tag this commit" or "delete this stale branch" are logical next steps after finding the right ref.
Features
Tag Management (Commit List)
Ref Management (Refs Panel)
Pending Overlay
Search & Filter
Hotkey Hints
Refresh
Bug Fixes
Performance
Technical Notes
Rc Migration: Originally,
&Commitand&Refwere passed as borrowed references — zero-cost, but inflexible. The problem: the tag creation dialog needs to hold commit data, but the commit list may refresh while the dialog is open.Rc<T>solves this — cloning just copies the pointer and increments the reference count, with no deep data copying.Why this is a good pattern:
Rc::clone()is O(1), just a pointer copy + atomic increment. No allocation, no memcpy of commit messages, hashes, etc.Rc<T>gives shared read-only access. If mutation is needed,Rc<RefCell<T>>makes it explicit.Rcis the standard escape hatch.Rcdrops. If no other references exist, memory is freed. No manual cleanup, no dangling pointers.Trade-off: Small runtime cost (refcount operations) vs. significant code simplicity. For UI data that's read frequently but rarely mutated, this is the right balance.
Ownership Change: App now owns Repository instead of borrowing it, enabling mutation for add_ref/remove_ref after tag/branch operations.
How Has This Been Tested?
Test repository generation with scripts/generate_test_repo.sh:
./scripts/generate_test_repo.sh /tmp/test-repo 10000
Creates realistic repository with feature/bugfix branches, merge commits, semver tags, multiple authors.
Tested scenarios:
I know the diff is large, but most changes are straightforward — tests pass and lints are clean. I believe this enriches the project without changing its core concept and principles. Happy to walk through any part if needed.