Skip to content

Commit 9f23b89

Browse files
authored
Improve hunk selection mode in staging view (#4684)
- **PR Description** Hunk selection mode is one of the features that many people don't know about, because it is not very discoverable. You can switch to it from line selection mode by pressing `a` in the staging view. The problem with this mode is that it selects entire hunks, where hunks are defined to be sections of the diff starting with `@@`. Very often, hunks consist of multiple distinct blocks of changes, separated by context lines. For example, with the default diff context size of 3 it takes at least 6 unchanged lines between blocks of changes for them to be separated into distinct hunks; if there are 5 or less unchanged lines between them, they are grouped into one hunk. And of course, if you increase the diff context size by pressing `}`, you will get even fewer hunks. Now, most of the time I want to navigate between the individual blocks of changes in a diff, regardless of how git groups them into hunks. That's what this PR does: when pressing `a`, the selection is extended to just the current group of changes, separated by context lines; you can easily stage it by pressing space, and the selection will move on to the next block of changes. Actual hunks no longer play a role here. Also, in line selection mode the right/left arrow keys now move between blocks of changes rather than actual hunks. I find this new behavior so useful that I almost always switch to hunk mode right away after entering the staging view. It saves a lot of keystrokes, since it is very rare that I want to select only some lines of a block of adjacent changes. This makes me wonder whether we should enable hunk mode by default when entering staging, but that's going to be another PR.
2 parents 3ea4cf0 + a6096f4 commit 9f23b89

File tree

6 files changed

+94
-136
lines changed

6 files changed

+94
-136
lines changed

.vscode/tasks.json

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,22 +24,24 @@
2424
{
2525
"label": "Run current file integration test",
2626
"type": "shell",
27-
"command": "go generate pkg/integration/tests/tests.go && go run cmd/integration_test/main.go cli ${relativeFile}",
27+
"command": "go run cmd/integration_test/main.go cli ${relativeFile}",
2828
"problemMatcher": [],
2929
"group": {
3030
"kind": "test",
3131
"isDefault": true
3232
},
3333
"presentation": {
34+
"clear": true,
3435
"focus": true
3536
}
3637
},
3738
{
3839
"label": "Run current file integration test (slow)",
3940
"type": "shell",
40-
"command": "go generate pkg/integration/tests/tests.go && go run cmd/integration_test/main.go cli --slow ${relativeFile}",
41+
"command": "go run cmd/integration_test/main.go cli --slow ${relativeFile}",
4142
"problemMatcher": [],
4243
"group": {
44+
"clear": true,
4345
"kind": "test",
4446
},
4547
"presentation": {
@@ -49,12 +51,13 @@
4951
{
5052
"label": "Run current file integration test (sandbox)",
5153
"type": "shell",
52-
"command": "go generate pkg/integration/tests/tests.go && go run cmd/integration_test/main.go cli --sandbox ${relativeFile}",
54+
"command": "go run cmd/integration_test/main.go cli --sandbox ${relativeFile}",
5355
"problemMatcher": [],
5456
"group": {
5557
"kind": "test",
5658
},
5759
"presentation": {
60+
"clear": true,
5861
"focus": true
5962
}
6063
},

pkg/gui/controllers/patch_explorer_controller.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -223,13 +223,13 @@ func (self *PatchExplorerController) HandleNextLineRange() error {
223223
}
224224

225225
func (self *PatchExplorerController) HandlePrevHunk() error {
226-
self.context.GetState().CycleHunk(false)
226+
self.context.GetState().SelectPreviousHunk()
227227

228228
return nil
229229
}
230230

231231
func (self *PatchExplorerController) HandleNextHunk() error {
232-
self.context.GetState().CycleHunk(true)
232+
self.context.GetState().SelectNextHunk()
233233

234234
return nil
235235
}

pkg/gui/patch_exploring/state.go

Lines changed: 66 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,11 @@ func (s *State) ToggleSelectHunk() {
125125
s.selectMode = LINE
126126
} else {
127127
s.selectMode = HUNK
128+
129+
// If we are not currently on a change line, select the next one (or the
130+
// previous one if there is no next one):
131+
s.selectedLineIdx = s.viewLineIndices[s.patch.GetNextChangeIdx(
132+
s.patchLineIndices[s.selectedLineIdx])]
128133
}
129134
}
130135

@@ -203,25 +208,49 @@ func (s *State) DragSelectLine(newSelectedLineIdx int) {
203208

204209
func (s *State) CycleSelection(forward bool) {
205210
if s.SelectingHunk() {
206-
s.CycleHunk(forward)
211+
if forward {
212+
s.SelectNextHunk()
213+
} else {
214+
s.SelectPreviousHunk()
215+
}
207216
} else {
208217
s.CycleLine(forward)
209218
}
210219
}
211220

212-
func (s *State) CycleHunk(forward bool) {
213-
change := 1
214-
if !forward {
215-
change = -1
221+
func (s *State) SelectPreviousHunk() {
222+
patchLines := s.patch.Lines()
223+
patchLineIdx := s.patchLineIndices[s.selectedLineIdx]
224+
nextNonChangeLine := patchLineIdx
225+
for nextNonChangeLine >= 0 && patchLines[nextNonChangeLine].IsChange() {
226+
nextNonChangeLine--
216227
}
217-
218-
hunkIdx := s.patch.HunkContainingLine(s.patchLineIndices[s.selectedLineIdx])
219-
if hunkIdx != -1 {
220-
newHunkIdx := hunkIdx + change
221-
if newHunkIdx >= 0 && newHunkIdx < s.patch.HunkCount() {
222-
start := s.patch.HunkStartIdx(newHunkIdx)
223-
s.selectedLineIdx = s.viewLineIndices[s.patch.GetNextChangeIdx(start)]
228+
nextChangeLine := nextNonChangeLine
229+
for nextChangeLine >= 0 && !patchLines[nextChangeLine].IsChange() {
230+
nextChangeLine--
231+
}
232+
if nextChangeLine >= 0 {
233+
// Now we found a previous hunk, but we're on its last line. Skip to the beginning.
234+
for nextChangeLine > 0 && patchLines[nextChangeLine-1].IsChange() {
235+
nextChangeLine--
224236
}
237+
s.selectedLineIdx = s.viewLineIndices[nextChangeLine]
238+
}
239+
}
240+
241+
func (s *State) SelectNextHunk() {
242+
patchLines := s.patch.Lines()
243+
patchLineIdx := s.patchLineIndices[s.selectedLineIdx]
244+
nextNonChangeLine := patchLineIdx
245+
for nextNonChangeLine < len(patchLines) && patchLines[nextNonChangeLine].IsChange() {
246+
nextNonChangeLine++
247+
}
248+
nextChangeLine := nextNonChangeLine
249+
for nextChangeLine < len(patchLines) && !patchLines[nextChangeLine].IsChange() {
250+
nextChangeLine++
251+
}
252+
if nextChangeLine < len(patchLines) {
253+
s.selectedLineIdx = s.viewLineIndices[nextChangeLine]
225254
}
226255
}
227256

@@ -259,11 +288,34 @@ func (s *State) CurrentHunkBounds() (int, int) {
259288
return start, end
260289
}
261290

291+
func (s *State) selectionRangeForCurrentBlockOfChanges() (int, int) {
292+
patchLines := s.patch.Lines()
293+
patchLineIdx := s.patchLineIndices[s.selectedLineIdx]
294+
295+
patchStart := patchLineIdx
296+
for patchStart > 0 && patchLines[patchStart-1].IsChange() {
297+
patchStart--
298+
}
299+
300+
patchEnd := patchLineIdx
301+
for patchEnd < len(patchLines)-1 && patchLines[patchEnd+1].IsChange() {
302+
patchEnd++
303+
}
304+
305+
viewStart, viewEnd := s.viewLineIndices[patchStart], s.viewLineIndices[patchEnd]
306+
307+
// Increase viewEnd in case the last patch line is wrapped to more than one view line.
308+
for viewEnd < len(s.patchLineIndices)-1 && s.patchLineIndices[viewEnd] == s.patchLineIndices[viewEnd+1] {
309+
viewEnd++
310+
}
311+
312+
return viewStart, viewEnd
313+
}
314+
262315
func (s *State) SelectedViewRange() (int, int) {
263316
switch s.selectMode {
264317
case HUNK:
265-
start, end := s.CurrentHunkBounds()
266-
return s.viewLineIndices[start], s.viewLineIndices[end]
318+
return s.selectionRangeForCurrentBlockOfChanges()
267319
case RANGE:
268320
if s.rangeStartLineIdx > s.selectedLineIdx {
269321
return s.selectedLineIdx, s.rangeStartLineIdx

pkg/integration/tests/patch_building/specific_selection.go

Lines changed: 3 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -55,31 +55,13 @@ var SpecificSelection = NewIntegrationTest(NewIntegrationTestArgs{
5555
).
5656
Press(keys.Main.ToggleSelectHunk).
5757
SelectedLines(
58-
Contains(`@@ -1,6 +1,6 @@`),
5958
Contains(`-1a`),
6059
Contains(`+aa`),
61-
Contains(` 1b`),
62-
Contains(`-1c`),
63-
Contains(`+cc`),
64-
Contains(` 1d`),
65-
Contains(` 1e`),
66-
Contains(` 1f`),
6760
).
6861
PressPrimaryAction().
6962
SelectedLines(
70-
Contains(`@@ -17,9 +17,9 @@`),
71-
Contains(` 1q`),
72-
Contains(` 1r`),
73-
Contains(` 1s`),
74-
Contains(`-1t`),
75-
Contains(`-1u`),
76-
Contains(`-1v`),
77-
Contains(`+tt`),
78-
Contains(`+uu`),
79-
Contains(`+vv`),
80-
Contains(` 1w`),
81-
Contains(` 1x`),
82-
Contains(` 1y`),
63+
Contains(`-1c`),
64+
Contains(`+cc`),
8365
).
8466
Tap(func() {
8567
t.Views().Information().Content(Contains("Building patch"))
@@ -154,8 +136,7 @@ var SpecificSelection = NewIntegrationTest(NewIntegrationTestArgs{
154136
Contains(`-1a`),
155137
Contains(`+aa`),
156138
Contains(` 1b`),
157-
Contains(`-1c`),
158-
Contains(`+cc`),
139+
Contains(` 1c`),
159140
Contains(` 1d`),
160141
Contains(` 1e`),
161142
Contains(` 1f`),

pkg/integration/tests/staging/diff_context_change.go

Lines changed: 0 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -52,67 +52,40 @@ var DiffContextChange = NewIntegrationTest(NewIntegrationTestArgs{
5252
IsFocused().
5353
Press(keys.Main.ToggleSelectHunk).
5454
SelectedLines(
55-
Contains(`@@ -1,6 +1,6 @@`),
56-
Contains(` 1a`),
57-
Contains(` 2a`),
5855
Contains(`-3a`),
5956
Contains(`+3b`),
60-
Contains(` 4a`),
61-
Contains(` 5a`),
62-
Contains(` 6a`),
6357
).
6458
Press(keys.Universal.IncreaseContextInDiffView).
6559
Tap(func() {
6660
t.ExpectToast(Equals("Changed diff context size to 4"))
6761
}).
6862
SelectedLines(
69-
Contains(`@@ -1,7 +1,7 @@`),
70-
Contains(` 1a`),
71-
Contains(` 2a`),
7263
Contains(`-3a`),
7364
Contains(`+3b`),
74-
Contains(` 4a`),
75-
Contains(` 5a`),
76-
Contains(` 6a`),
77-
Contains(` 7a`),
7865
).
7966
Press(keys.Universal.DecreaseContextInDiffView).
8067
Tap(func() {
8168
t.ExpectToast(Equals("Changed diff context size to 3"))
8269
}).
8370
SelectedLines(
84-
Contains(`@@ -1,6 +1,6 @@`),
85-
Contains(` 1a`),
86-
Contains(` 2a`),
8771
Contains(`-3a`),
8872
Contains(`+3b`),
89-
Contains(` 4a`),
90-
Contains(` 5a`),
91-
Contains(` 6a`),
9273
).
9374
Press(keys.Universal.DecreaseContextInDiffView).
9475
Tap(func() {
9576
t.ExpectToast(Equals("Changed diff context size to 2"))
9677
}).
9778
SelectedLines(
98-
Contains(`@@ -1,5 +1,5 @@`),
99-
Contains(` 1a`),
100-
Contains(` 2a`),
10179
Contains(`-3a`),
10280
Contains(`+3b`),
103-
Contains(` 4a`),
104-
Contains(` 5a`),
10581
).
10682
Press(keys.Universal.DecreaseContextInDiffView).
10783
Tap(func() {
10884
t.ExpectToast(Equals("Changed diff context size to 1"))
10985
}).
11086
SelectedLines(
111-
Contains(`@@ -2,3 +2,3 @@`),
112-
Contains(` 2a`),
11387
Contains(`-3a`),
11488
Contains(`+3b`),
115-
Contains(` 4a`),
11689
).
11790
PressPrimaryAction().
11891
Press(keys.Universal.TogglePanel)
@@ -121,18 +94,14 @@ var DiffContextChange = NewIntegrationTest(NewIntegrationTestArgs{
12194
IsFocused().
12295
Press(keys.Main.ToggleSelectHunk).
12396
SelectedLines(
124-
Contains(`@@ -2,3 +2,3 @@`),
125-
Contains(` 2a`),
12697
Contains(`-3a`),
12798
Contains(`+3b`),
128-
Contains(` 4a`),
12999
).
130100
Press(keys.Universal.DecreaseContextInDiffView).
131101
Tap(func() {
132102
t.ExpectToast(Equals("Changed diff context size to 0"))
133103
}).
134104
SelectedLines(
135-
Contains(`@@ -3,1 +3 @@`),
136105
Contains(`-3a`),
137106
Contains(`+3b`),
138107
).
@@ -141,24 +110,16 @@ var DiffContextChange = NewIntegrationTest(NewIntegrationTestArgs{
141110
t.ExpectToast(Equals("Changed diff context size to 1"))
142111
}).
143112
SelectedLines(
144-
Contains(`@@ -2,3 +2,3 @@`),
145-
Contains(` 2a`),
146113
Contains(`-3a`),
147114
Contains(`+3b`),
148-
Contains(` 4a`),
149115
).
150116
Press(keys.Universal.IncreaseContextInDiffView).
151117
Tap(func() {
152118
t.ExpectToast(Equals("Changed diff context size to 2"))
153119
}).
154120
SelectedLines(
155-
Contains(`@@ -1,5 +1,5 @@`),
156-
Contains(` 1a`),
157-
Contains(` 2a`),
158121
Contains(`-3a`),
159122
Contains(`+3b`),
160-
Contains(` 4a`),
161-
Contains(` 5a`),
162123
)
163124
},
164125
})

0 commit comments

Comments
 (0)