Skip to content

Commit 6160717

Browse files
poteatpoteat
authored
diff: the dialog opens every file it lists, and says nothing when closed (#98555)
* diff: the dialog opens every file it lists, and says nothing when closed * diff: the dialog's fetch asks its seat first, and a generated file opens too * diff: a read started by a move of the selection never rejects unheard * diff: a test that a moving window reads only its new rows
1 parent 525d3b3 commit 6160717

6 files changed

Lines changed: 152 additions & 23 deletions

File tree

‎mods/diff/hooks/drawn-files-of/drawn-files-of.ts‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,35 @@
11
import type Git from '../git'
22
import Limits from '../limits'
33
import PaneState from '../pane-state'
4+
import Views from '../views'
45

56
/**
6-
* The files whose bodies the pane draws now, in drawing order: the listed
7-
* session rows, then the pre-session rows while their section is open.
7+
* The files whose bodies the pane draws or a press opens: every row its
8+
* seat lists now.
89
*
9-
* Past PRE_SESSION_BODY_CAP pre-session files only their legend shows, so
10-
* none of their bodies is wanted.
10+
* Docked, the listed session rows, then the pre-session rows while their
11+
* section is open, none past PRE_SESSION_BODY_CAP. Inline, the rows in the
12+
* dialog's window, none for a turn, whose rows carry their own bodies.
1113
*
12-
* @param model the pane's state: its fetch and its two toggles
14+
* @param model the pane's state: its fetch, its seat, its toggles, its pick
1315
* @returns the rows, none without a fetch
1416
*/
1517
export function drawnFilesOf(
16-
model: Pick<
17-
PaneState.PaneModel,
18-
'data' | 'isNoiseShown' | 'isPreSessionShown'
19-
>,
18+
model: PaneState.PaneModel,
2019
): readonly Git.FileStat[] {
20+
const files = model.data?.files ?? []
21+
22+
if (model.placement === 'inline') {
23+
const isTurn = PaneState.pickedTurnOf(model) !== undefined
24+
const listed = Views.dialogEntriesOf(model).map(entry => entry.path)
25+
const start = Views.dialogWindowOf(model, listed)
26+
const windowed = listed.slice(start, start + Limits.MAX_VISIBLE_FILES)
27+
28+
return files.filter(file => !isTurn && windowed.includes(file.path))
29+
}
30+
2131
const partition = PaneState.partitionOf(
22-
model.data?.files ?? [],
32+
files,
2333
model.isNoiseShown ? 'shown' : 'hidden',
2434
)
2535

‎mods/diff/hooks/register.ts‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -544,6 +544,7 @@ export function register(on: On) {
544544
place: isDocked ? Views.placeAtFile(model, path) : model.place,
545545
}
546546

547+
void loadBodies(engine).catch(() => undefined)
547548
redraw(engine)
548549
},
549550
scrollList: delta => {
@@ -727,6 +728,15 @@ export function register(on: On) {
727728

728729
columns = e.viewport?.columns ?? columns
729730

731+
/**
732+
* A seat that changed since the last drawing lists other rows, whose
733+
* bodies are read once.
734+
*
735+
* The model takes the new seat in this same pass, so the drawing that
736+
* read asks for finds the seat unchanged.
737+
*/
738+
const isReseated = e.props.placement !== model.placement
739+
730740
model = {
731741
...model,
732742
placement: e.props.placement,
@@ -740,6 +750,10 @@ export function register(on: On) {
740750
},
741751
}
742752

753+
if (isReseated) {
754+
void loadBodies(host).catch(() => undefined)
755+
}
756+
743757
return Views.paneView(
744758
{
745759
ui: { Box, Text, Button, Select, Code },
@@ -830,13 +844,9 @@ export function register(on: On) {
830844
isPaneOpen = false
831845
}
832846

833-
const isDialog = model.isFullscreen === false
834-
835-
if (isPersons && host && isDialog) {
836-
host.uiLog(Names.DIALOG_DISMISSED_TEXT)
837-
}
847+
const isDocking = model.isFullscreen !== false
838848

839-
if (isPersons && host && !isDialog) {
849+
if (isPersons && host && isDocking) {
840850
markTabSwitch(host, 'convo')
841851
await host.storeSet(Names.STORE_OPEN_KEY, false).catch(() => undefined)
842852
}
@@ -859,6 +869,7 @@ export function register(on: On) {
859869
}
860870

861871
model = { ...model, selectedPath: focus.selectedPath }
872+
void loadBodies(host).catch(() => undefined)
862873
fitDialog(host)
863874
host.invalidate()
864875

‎mods/diff/tests/fixtures/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ export * from './usage-at.js'
6161
export * from './vs-main.js'
6262
export * from './wheel-over-list.js'
6363
export * from './wheel-tick.js'
64+
export * from './with-lockfile.js'
6465
export * from './worktree'
6566
export * from './wrapped-lines'
6667

‎mods/diff/tests/fixtures/old-files.ts‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,23 @@
11
import type { On } from 'claude-code'
22

33
/**
4-
* Answers `$.fs` over /work holding MOVED_IN's two files, both last
5-
* written at time 0, before any session began.
4+
* Answers `$.fs` over /work holding the named files, MOVED_IN's two when
5+
* none is given, each last written at time 0, before any session began.
66
*
77
* @param on the test's `on`
8+
* @param names the files /work holds
89
*/
9-
export function oldFiles(on: On) {
10+
export function oldFiles(
11+
on: On,
12+
names: readonly string[] = ['old.ts', 'moved.ts'],
13+
) {
1014
on('fs.list', () => ({
11-
value: [
12-
{ name: 'old.ts', kind: 'file', size: 2, isLink: false },
13-
{ name: 'moved.ts', kind: 'file', size: 2, isLink: false },
14-
],
15+
value: names.map(name => ({
16+
name,
17+
kind: 'file' as const,
18+
size: 2,
19+
isLink: false,
20+
})),
1521
}))
1622

1723
on('fs.stat', () => ({

‎mods/diff/tests/fixtures/with-lockfile.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
import { answersOf } from './answers-of.js'
2+
3+
/**
4+
* Git's output in /work where a source file and a lockfile changed, the
5+
* lockfile a generated file the docked pane hides until asked.
6+
*/
7+
export const WITH_LOCKFILE = answersOf('1\t1\tapp.ts\0' + '1\t1\tbun.lock\0', {
8+
'app.ts': '@@ -1 +1 @@\n-const a = 1\n+const a = 2\n',
9+
'bun.lock': '@@ -1 +1 @@\n-"left": "1.0.0"\n+"left": "1.0.1"\n',
10+
})

‎mods/diff/tests/views.test.ts‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,97 @@ describe('views', () => {
9292
expect(drawn).toContain('\u2191/\u2193 to scroll \u00b7 Esc to back')
9393
})
9494

95+
test('inline, a file edited before the session opens too', async ($, on) => {
96+
const world = Fixtures.inRepository(on, Fixtures.MOVED_IN)
97+
98+
on('session.usage', () => ({ value: Fixtures.usageAt(Fixtures.SETTLE_MS) }))
99+
Fixtures.oldFiles(on)
100+
101+
await $.session.start(Fixtures.SESSION)
102+
await $.command.run(Fixtures.DIALOG_DIFF)
103+
await world.clock.advance(Fixtures.SETTLE_MS)
104+
105+
expect(
106+
Fixtures.textOf(await $.ui.render(Fixtures.INLINE_PANE)),
107+
"the dialog lists it beside the session's file",
108+
).toContain('old.ts')
109+
110+
expect(await $.ui.press({ plugin: 'diff', key: 'file:old.ts' })).toEqual({
111+
element: 'file:old.ts',
112+
})
113+
114+
await world.clock.advance(Fixtures.SETTLE_MS)
115+
116+
const drawn = Fixtures.textOf(await $.ui.render(Fixtures.INLINE_PANE))
117+
118+
expect(
119+
drawn,
120+
'a listed row can be pressed, so its hunks were read',
121+
).not.toContain('Loading diff')
122+
123+
expect(drawn).toContain('+b')
124+
expect(drawn).not.toContain('+d')
125+
})
126+
127+
test('inline, a file the docked pane hides opens too', async ($, on) => {
128+
const world = Fixtures.inRepository(on, Fixtures.WITH_LOCKFILE)
129+
130+
await $.session.start(Fixtures.SESSION)
131+
await $.command.run(Fixtures.DIALOG_DIFF)
132+
await world.clock.advance(Fixtures.SETTLE_MS)
133+
await $.ui.render(Fixtures.INLINE_PANE)
134+
135+
expect(await $.ui.press({ plugin: 'diff', key: 'file:bun.lock' })).toEqual({
136+
element: 'file:bun.lock',
137+
})
138+
139+
await world.clock.advance(Fixtures.SETTLE_MS)
140+
141+
const drawn = Fixtures.textOf(await $.ui.render(Fixtures.INLINE_PANE))
142+
143+
expect(drawn, 'generated, and still a listed row').not.toContain(
144+
'Loading diff',
145+
)
146+
147+
expect(drawn).toContain('+"left": "1.0.1"')
148+
})
149+
150+
test('inline, a window that moves reads only its new rows', async ($, on) => {
151+
const world = Fixtures.inRepository(on, Fixtures.MANY_FILES)
152+
153+
on('session.usage', () => ({ value: Fixtures.usageAt(Fixtures.SETTLE_MS) }))
154+
155+
Fixtures.oldFiles(
156+
on,
157+
Array.from(
158+
{ length: Fixtures.MANY_FILE_COUNT },
159+
(_, at) => `file${at}.ts`,
160+
),
161+
)
162+
163+
await $.session.start(Fixtures.SESSION)
164+
await $.command.run(Fixtures.DIALOG_DIFF)
165+
await world.clock.advance(Fixtures.SETTLE_MS)
166+
await $.ui.render(Fixtures.INLINE_PANE)
167+
await world.clock.advance(Fixtures.SETTLE_MS)
168+
await $.ui.focus(Fixtures.ringOnto('file:file3.ts'))
169+
await world.clock.advance(Fixtures.SETTLE_MS)
170+
await $.ui.render(Fixtures.INLINE_PANE)
171+
await $.ui.press({ plugin: 'diff', key: 'file:file5.ts' })
172+
await world.clock.advance(Fixtures.SETTLE_MS)
173+
174+
expect(
175+
world.runs
176+
.filter(run => run.argv.includes('--raw'))
177+
.map(run => run.argv.slice(run.argv.indexOf('--') + 1).join(' ')),
178+
'the first five, the one the walk brought in, the two the press did',
179+
).toEqual([
180+
'file0.ts file1.ts file2.ts file3.ts file4.ts',
181+
'file5.ts',
182+
'file6.ts file7.ts',
183+
])
184+
})
185+
95186
test('docked, a wheel tick moves the body, not the list', async ($, on) => {
96187
const world = Fixtures.inRepository(on, Fixtures.MANY_FILES)
97188

0 commit comments

Comments
 (0)