Skip to content

feat(hunk-mode): Add hunk mode (add to stage & revert block actions) #17

Closed
fcmiranda wants to merge 1 commit into
Blankeos:mainfrom
fcmiranda:feature/hunk-mode
Closed

feat(hunk-mode): Add hunk mode (add to stage & revert block actions) #17
fcmiranda wants to merge 1 commit into
Blankeos:mainfrom
fcmiranda:feature/hunk-mode

Conversation

@fcmiranda
Copy link
Copy Markdown
Contributor

@fcmiranda fcmiranda commented May 16, 2026

Summary

Merges feature/revert-block-clean into feature/hunk-mode, introducing a modal hunk mode for staging and reverting individual diff hunks via keyboard.

screenrecording-2026-05-16_18-59-35.mp4

Hunk mode

Press H in the Files panel to enter hunk mode. While active:

Key Action
H Enter hunk mode (also focuses the diff)
j / k Cycle to next / previous hunk
a Stage the selected hunk
r Revert the selected hunk
u Undo the last hunk action (session-scoped)
esc Exit hunk mode (first press), then clear selection, then clear search, then unfocus diff

Mouse clicking a hunk marker selects it — then use a / r to act on it.

What's included

  • Modal hunk navigationH enters hunk mode; j/k cycle hunks; esc exits. Replaces the old {/} non-modal cycling.
  • Stage hunk (a) — stages the selected hunk from the worktree into the index via git apply --cached
  • Revert hunk (r) — reverts the selected hunk in the worktree via git apply --reverse
  • Patch-based undo (u) — undo stack stores the patch text + metadata; undo re-applies the inverse patch instead of restoring file bytes
  • Sticky hunk marker — marker stays pinned to the first visible row of its hunk span while scrolling
  • Hunk centering — selected hunk is recentered after navigation and after a diff refresh
  • Selection preserved after action — nearest remaining hunk is auto-selected on refresh
  • Hunk marker styling — default icon (blends with the divider), bold on by default; selected state shows ``, hovered shows accent-secondary color. Colors follow the active border color by default.
  • Configurable keybindingsenterHunkMode, stageBlock, revertBlock, undoRevertBlock are all user-configurable in keybindings.universal
  • Legend scoped correctly — hunk mode hints appear only in the Files panel and diff panel, not in Branches/Commits/etc.
  • Restored reset_keep_prefs() — dropped in main but still called by the diff-mode controller; preserves wrap/side-view prefs across navigation

Removed

  • {/} prev/next hunk keybindings (prevRevertBlock / nextRevertBlock) — hunk cycling is now exclusively j/k inside hunk mode

Conflict resolutions (hunk-mode priority)

File Resolution
src/config/keybindings.rs Hunk-mode keybindings; removed {/} fields
src/gui/mod.rs Hunk-mode dispatch; matches_key over hardcoded KeyCode handlers
src/gui/presentation/diff_mode.rs Hunk-marker rendering
src/gui/views.rs Status bar hints scoped to Files/diff context only
src/pager/side_by_side.rs HunkMarkerStyle, HunkActionKind, patch-based undo; restored reset_keep_prefs()

@fcmiranda fcmiranda force-pushed the feature/hunk-mode branch from 338529b to adef2f5 Compare May 16, 2026 19:57
@fcmiranda fcmiranda changed the title feat(hunk-mode): integrate revert-block improvements with hunk-mode priority feat(hunk-mode): Add hunk mode ( add to stage & revert block actions) May 16, 2026
@fcmiranda fcmiranda changed the title feat(hunk-mode): Add hunk mode ( add to stage & revert block actions) feat(hunk-mode): Add hunk mode (add to stage & revert block actions) May 16, 2026
@Blankeos
Copy link
Copy Markdown
Owner

Cool! Tested it well. The new concepts are pretty good.

  1. Removing the Hunk cycling with {/} might not be a good idea. It's not exclusive to "hunk mode" and it's kind of essential outside of it to jump between hunks. Lets...

    • Keep the {/} to navigate outside of hunk mode
    • (Consider) Keep the {/} to navigate inside of hunk mode, also essentially make it cycle the hunks (so it's like an alternative to j/k, implicit "cycle hunks" - just like how and is right now).
  2. In hunk mode I see two j/k entries in the help footer (Maybe remove 1?)

image

Other than that, looking good. Thanks for the additions! The PR looks a little big tho, I noticed there are some commits from a previous PR or something, maybe needs cleanup, rebase, or whatever. But I trust you. Thanks!

@fcmiranda
Copy link
Copy Markdown
Contributor Author

@Blankeos Thank you for the feedback! I'm gonna work on this PR this week :)

@fcmiranda fcmiranda force-pushed the feature/hunk-mode branch 2 times, most recently from 31ca81c to 29dfe08 Compare May 19, 2026 02:02
@fcmiranda
Copy link
Copy Markdown
Contributor Author

fcmiranda commented May 19, 2026

Update: hunk mode removed, {/} is now always-on

After review, the explicit hunk mode (toggled with H) has been removed in favor of a simpler, always-on design:

What changed

  • {/} always behaves like hunk mode did — navigating with { / } now always selects the hunk, centers the diff view on it, and wraps around. No mode to enter first.
  • H key removed — there is no longer a separate "hunk mode" toggle.
  • j/k scroll normally — they no longer cycle hunks; use {/} exclusively for hunk navigation.
  • esc clears hunk selection — instead of "exiting hunk mode".
  • Status bar hints simplified{/} shows as a persistent hint when hunks are available; duplicate j/k hint when in hunk mode is gone.
  • Help popup updated — removed "Enter hunk mode" entries, updated descriptions for {/} and esc.

Bug fixes included

  • Fixed matches_key SHIFT modifier bug that prevented { / } from being recognized when the terminal sends them with KeyModifiers::SHIFT (keyboard enhancement protocol).
  • Fixed {/} being intercepted early in handle_diff_focused_key before the raw KeyCode::Char handlers were reached (so they now work correctly when diff is focused).
  • Fixed {/} in the sidebar-focused path (previously only worked when diff was focused).

Internals cleaned up

  • Removed hunk_mode: bool field from DiffViewState
  • Removed enter_hunk_mode from UniversalKeybinding
  • All 22 original commits squashed into 1 clean commit on top of v0.0.20
image

- Add {/} always-on hunk navigation: selects hunk, centers view, wraps around
- Add hunk action markers in diff view (stage/revert tooltips)
- Add HunkActionKind, HunkActionUndoEntry for undo stack
- Add HunkMarkerStyle for customizable marker appearance
- Add stage hunk (a), revert hunk (r), undo last hunk action (u) keybindings
- Fix matches_key SHIFT modifier bug for non-alphabetic keys (e.g. {/})
- Remove explicit hunk mode (H key) — {/} always behaves like hunk mode
- Improve diff layout: divider width, content rendering adjustments
- Add smoke tests for hunk parsing
@fcmiranda fcmiranda force-pushed the feature/hunk-mode branch from 29dfe08 to 5816bd6 Compare May 19, 2026 02:10
@Blankeos
Copy link
Copy Markdown
Owner

Blankeos commented May 19, 2026

Hi fcmiranda! I really appreciate the changes! Sorry to be critical again...

But have you checked 0.0.20 lately? Since you removed hunk mode. The current state of your PR just looks almost like it, just without the enter to view the Hunk Actions dialog, and replaced with a to add and r to revert.

image
0.0.20-demo.mp4

If the main purpose of this PR is just to add "Add hunk to staging"... Then I can imagine, that could just be a new item under the Hunk Actions dialog.


What I don't like.

  1. (UI/UX) The ⚡ icon is vertically centered in the diff. And the icon is the focus.
image

On a small window height with long diffs, the "first line" of the diff gets cut off, in favor of focusing the middle section. This is related to point 2.

  1. (UX) The {/} cycling is completely replaced with this "window-based" scrolling (the old c-j and c-k behavior). I actually liked the drastic scrolling done by {/} because it always puts the next diff's first line at the very top. This is what I like about the current state in 0.0.20

  2. (Bug) "Adding a hunk to staging", removes it from the diff view panel. I'm sure this is a bug that we should definitely fix.

  3. (Bug) Unclean PR + introduces some major regressions on bugs I fixed before, maybe because of merge conflict resolution attempts?

**P1 - Diff preferences are no longer preserved when switching selections.**

   Several refresh paths now assign `self.diff_view = DiffViewState::new()` when the selected diff changes, for example `src/gui/mod.rs:1064` and `src/gui/mod.rs:1090`. The existing helper `DiffViewState::reset_keep_prefs()` at `src/pager/side_by_side.rs:360` explicitly preserves user-controlled prefs such as wrap and side-view. With the new assignments, toggling wrap or old/new-only view gets reset during normal file navigation. Use `reset_keep_prefs()` for selection changes and clear only the new hunk-selection state that should not survive.

**P2 - Commit body `Enter` behavior appears to regress.**

   The branch changes newline insertion to only Shift+Enter/Ctrl+J at `src/gui/mod.rs:2608`, and the fallback body key handler at `src/gui/mod.rs:2733` does not handle plain `KeyCode::Enter`. That means pressing Enter while focused in the commit description no longer inserts a newline, whereas the previous branch behavior explicitly allowed that. If this is intentional, the UI/help should say so; otherwise restore Enter-in-body newline insertion.

**Compare/Diff Mode Hunk Navigation Regression**

  - Before: in compare/diff mode, { and } called prev_hunk() / next_hunk().
  - After: those match arms were removed from src/gui/controller/diff_mode.rs:498, so { / } no longer navigate hunks in Diff/Compare mode.

**P0 - `cargo test` cannot compile because `tests/smoke.rs` contains raw tokens.**

   `tests/smoke.rs:111`, `tests/smoke.rs:217`, `tests/smoke.rs:220`, `tests/smoke.rs:225`, and later lines contain bare text like `si`, `add`, `use`, `hihihihi`, etc. Rust parses `si` as the start of an item path, then fails at `add` with `expected one of ! or ::`. This blocks CI if it runs tests or all targets. Remove the junk content or replace this file with real integration coverage before merging.

Btw I found those issues in Point 4 with a simple /pr-review command. If you use an agent to code. It's like using Greptile/Coderabbit w/ your favorite agent. This is my personal pr-review command: https://github.com/Blankeos/dotfiles/blob/main/.claude/commands/pr-review.md


What I like:

  • ✅ Simplifying it and just using {/} to navigate, no more c-j/k What could be better (for me): Stick w/ the old behavior of keeping the jump at the first line of the diff, not vertically centered. Also with the icon, make it at the first line of the diff.
  • ✅ Adding those a and r shortcuts, so there's no need to open via a Dialog... The dialog has its place though, it's simple, less learning curve, less mental load for the user.
  • ✅ "Add to staging" is a great addition (which I guess the main purpose of this PR).
    • What could be better: Tbh, just addressing the point 3 (bug). I think what the original lazygit actually does is it has a subdivision of "Staged changes" and "Unstaged" changes. (I personally don't like this at all, it takes too much space).
      image
      • My current idea is to just add a new separate color for "staged" changes, or give it an "A" label in the gutter.
        image

In any case...

I really appreciate the contributions! I wouldn't want to bother you too... So if you're okay with it--I can just branch off of this PR and work on it further? Just let me know, thanks!

@fcmiranda
Copy link
Copy Markdown
Contributor Author

@Blankeos, yes for sure! I'm sorry about the inconsistencies on the last PR update. Thanks for be critic, I like that, I prefer in this way to make better contributions in the future. Yes I like your idea of the small "a". Please go ahead you can branch this PR and finish it! Thanks for the pr-review tip I'm gonna add a skill or something for that :)

@fcmiranda fcmiranda closed this May 20, 2026
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.

2 participants