Skip to content

Commit d017bd6

Browse files
claudeclaude
committed
Merge remote-tracking branch 'origin/main' into claude/diff-pane-empty-auto-open
# Conflicts: # mods/diff/README.md # mods/diff/hooks/register.ts # mods/diff/tests/register.test.ts
2 parents d7f15cd + 92ec78f commit d017bd6

5 files changed

Lines changed: 103 additions & 12 deletions

File tree

‎mods/diff/README.md‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,10 @@ before; a person who closed it is left alone) and file checkpointing is on;
2222
a subagent's edit opens nothing, and where the surface does not say,
2323
nothing opens by itself. An edit to a file outside the repository, or one
2424
after which the diff lists nothing or cannot be read, opens nothing and
25-
leaves the opening to a later edit. An open the engine leaves waiting
26-
undrawn is withdrawn, so no later resize seats it, and the next edit asks
27-
again.
25+
leaves the opening to a later edit. A docked pane fetches before it opens,
26+
as the built-in panel primes its data, so it never lands on
27+
`Loading diff…`; an open the engine leaves waiting undrawn is withdrawn, so
28+
no later resize seats it, and the next edit asks again.
2829

2930
Under the fullscreen layout a terminal under 110 columns gets the
3031
built-in's line asking for a wider one and nothing opens. Without that

‎mods/diff/hooks/register.ts‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,8 @@ import Views from './views'
3535
*
3636
* Git runs when the built-in's would: `session.start` binds the host and
3737
* registers `/diff`; `/diff` or the main loop's first checkpointed edit with
38-
* room pins the backend, until `/clear`; a placed, open pane fetches, and so
39-
* does such an edit inside the tree, until one lists a file to open on.
38+
* room pins the backend, until `/clear`; a docked pane fetches, then opens,
39+
* and such an edit inside the tree fetches until one lists a file to open on.
4040
*
4141
* @param on the engine's registrar
4242
*/
@@ -430,8 +430,12 @@ export function register(on: On) {
430430

431431
dialogRows = isDialog ? Views.dialogRowsOf(model) : null
432432

433+
const landedBefore = landed
434+
433435
if (read) {
434436
model = PaneState.afterFetch(model, read)
437+
} else if (!isDialog) {
438+
await refresh(engine).catch(() => undefined)
435439
}
436440

437441
const opened = await engine.openPane(
@@ -457,7 +461,11 @@ export function register(on: On) {
457461
Record.recorderOf(engine).shown(trigger, Record.widthBucketOf(columns))
458462
}
459463

460-
void refresh(engine, read)
464+
const isStale = isDialog || read !== null || landed !== landedBefore
465+
466+
if (isStale) {
467+
void refresh(engine, read)
468+
}
461469

462470
return true
463471
}
@@ -953,7 +961,7 @@ export function register(on: On) {
953961
landed += 1
954962
}
955963

956-
if (isPaneOpen && hasLanded) {
964+
if (hasLanded && isPaneOpen) {
957965
scheduleRefresh(engine)
958966
}
959967

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
export type * from './beneath.js'
22
export * from './left-waiting.js'
3+
export * from './slow-diff-ms.js'
34

45
export * as default from '.'

‎mods/diff/tests/fixtures/beneath/slow-diff-ms.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
/**
2+
* How long a slow repository takes to answer `git diff` in the tests that
3+
* time the pane's opening against its first fetch: well inside git's timeout.
4+
*/
5+
export const SLOW_DIFF_MS = 3000

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

Lines changed: 81 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -784,9 +784,14 @@ describe('register', () => {
784784

785785
expect(world.reads, "the edit's read is under way").toEqual([0])
786786

787-
expect(await $.command.run(Fixtures.DIFF)).toEqual({
788-
text: 'Diff panel shown',
789-
})
787+
const shown = $.command.run(Fixtures.DIFF)
788+
789+
await world.clock.advance(1)
790+
791+
expect(
792+
await shown,
793+
'/diff reads before it opens, as the edit does',
794+
).toEqual({ text: 'Diff panel shown' })
790795

791796
expect(await $.command.run(Fixtures.DIFF)).toEqual({
792797
text: 'Diff panel hidden',
@@ -816,7 +821,11 @@ describe('register', () => {
816821
})
817822

818823
await world.clock.settle()
819-
await $.command.run(Fixtures.DIFF)
824+
825+
const shown = $.command.run(Fixtures.DIFF)
826+
827+
await world.clock.advance(1)
828+
await shown
820829
await $.command.run(Fixtures.DIFF)
821830
await $.tool.call({ tool: 'Bash', command: 'make' })
822831
await world.clock.advance(Fixtures.SETTLE_MS)
@@ -843,8 +852,11 @@ describe('register', () => {
843852
})
844853

845854
await world.clock.settle()
846-
await $.command.run(Fixtures.DIFF)
855+
856+
const shown = $.command.run(Fixtures.DIFF)
857+
847858
await world.clock.advance(Fixtures.SETTLE_MS)
859+
await shown
848860

849861
expect(
850862
world.opened.map(pane => pane.id),
@@ -1029,6 +1041,12 @@ describe('register', () => {
10291041
'and withdrew the pane the engine left waiting, so no resize seats it',
10301042
).toEqual(['diff'])
10311043

1044+
expect(
1045+
world.runs.map(run => Fixtures.gitWordOf(run.argv)).slice(0, 2),
1046+
'the repository was found and read once before the open, as the ' +
1047+
'built-in primes; nothing polls for a pane no one sees',
1048+
).toEqual(['rev-parse --show-toplevel', 'status'])
1049+
10321050
expect(
10331051
world.runs.map(run => Fixtures.gitWordOf(run.argv)),
10341052
'the repository was found and read once to decide, no hunks fetched ' +
@@ -1041,6 +1059,11 @@ describe('register', () => {
10411059
'ls-files',
10421060
])
10431061

1062+
expect(
1063+
world.runs.map(run => Fixtures.gitWordOf(run.argv)),
1064+
'no HEAD poll for a withdrawn pane',
1065+
).not.toContain(Fixtures.POLL_WORD)
1066+
10441067
isNarrow = false
10451068

10461069
expect(
@@ -1094,6 +1117,59 @@ describe('register', () => {
10941117
).toEqual(['diff'])
10951118
})
10961119

1120+
test('a docked pane opens once its first fetch settled', async ($, on) => {
1121+
const opened: string[] = []
1122+
const clock = Fixtures.startsSession(on)
1123+
1124+
on('process.run', async ($, e) => {
1125+
if (e.argv.includes('--shortstat')) {
1126+
await clock.sleep(Fixtures.SLOW_DIFF_MS)
1127+
}
1128+
1129+
return { value: Fixtures.gitIn(e.argv) }
1130+
})
1131+
1132+
on('ui.open', ($, e) => {
1133+
opened.push(e.id)
1134+
1135+
return { value: undefined }
1136+
})
1137+
1138+
on('ui.close', () => ({ value: undefined }))
1139+
on('ui.invalidate', () => ({ value: undefined }))
1140+
on('ui.render', { component: 'PromptHint' }, () => Fixtures.HINT_DRAWN)
1141+
on('session.messages', () => ({ value: [] }))
1142+
on('settings.read', () => ({ value: {} }))
1143+
on('tool.call', () => ({ result: 'edited' }))
1144+
mock.store(on)
1145+
mock.env(on, {})
1146+
1147+
await $.session.start(Fixtures.SESSION)
1148+
await $.ui.render(Fixtures.HINT)
1149+
1150+
await $.tool.call({
1151+
tool: 'Edit',
1152+
file_path: '/work/app.ts',
1153+
old_string: '1',
1154+
new_string: '2',
1155+
})
1156+
1157+
await clock.advance(Fixtures.SLOW_DIFF_MS - 1)
1158+
1159+
expect(opened, 'git has not answered: no pane, no Loading frame').toEqual(
1160+
[],
1161+
)
1162+
1163+
await clock.advance(Fixtures.SETTLE_MS)
1164+
1165+
expect(opened, 'the fetch settled: the pane opens filled').toEqual(['diff'])
1166+
1167+
const drawn = Fixtures.textOf(await $.ui.render(Fixtures.PANE))
1168+
1169+
expect(drawn).toContain('1 file changed')
1170+
expect(drawn).not.toContain('Loading diff')
1171+
})
1172+
10971173
test('/clear closes the pane it finds open', async ($, on) => {
10981174
const world = Fixtures.inRepository(on)
10991175

0 commit comments

Comments
 (0)