Skip to content

Commit d9c0beb

Browse files
committed
fix(sidebar): keep linked worktree rows as leaf nodes
Linked worktrees should never become alternate parents for a worktree family in the sidebar. After the earlier synthetic-row support, orphan linked worktrees could still synthesize sibling children underneath themselves, which made grouping unstable and produced confusing results when a saved linked worktree also had custom alias or group metadata. Changes: - stop synthesizing sibling worktree rows underneath orphan linked worktree entries so only main worktrees can own nested children - use the linked worktree alias when present, otherwise fall back to the worktree basename, so saved linked rows can keep their explicit label without becoming a second group root - treat both saved and synthetic linked worktree rows as `Delete…` targets in the sidebar context menu while keeping alias and group actions available only for saved rows - extend the delete-worktree popup payload so deleting a saved linked worktree also removes its stored Desktop repository entry, and so deleting the currently selected synthetic worktree still switches away safely before removal - add regression coverage for orphan linked rows staying flat and for the saved-vs-virtual linked worktree context menu behavior Behavioral effect: Known linked worktrees now remain leaf rows unless they are displayed under their actual main worktree. Saved linked worktrees no longer sprout sibling children under custom groups, and deleting either a saved or synthetic linked worktree uses the same linked-worktree action while keeping Desktop's repository store in sync. Testing: - yarn test:unit app/test/unit/repositories-list-grouping-test.ts - yarn test:unit app/test/unit/repository-list-item-context-menu-test.ts - yarn eslint app/src/ui/repositories-list/worktree-list-items.ts app/src/ui/repositories-list/repository-list-item-context-menu.ts app/src/ui/repositories-list/repositories-list.tsx app/src/ui/worktrees/delete-worktree-dialog.tsx app/src/ui/app.tsx app/src/models/popup.ts app/test/unit/repositories-list-grouping-test.ts app/test/unit/repository-list-item-context-menu-test.ts - yarn compile:prod
1 parent 43a2ddc commit d9c0beb

8 files changed

Lines changed: 165 additions & 15 deletions

app/src/models/popup.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -513,5 +513,7 @@ export type PopupDetail =
513513
type: PopupType.DeleteWorktree
514514
repository: Repository
515515
worktreePath: string
516+
storedRepositoryToRemove?: Repository
517+
isDeletingCurrentWorktree?: boolean
516518
}
517519
export type Popup = IBasePopup & PopupDetail

app/src/ui/app.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2763,6 +2763,8 @@ export class App extends React.Component<IAppProps, IAppState> {
27632763
key="delete-worktree"
27642764
repository={popup.repository}
27652765
worktreePath={popup.worktreePath}
2766+
storedRepositoryToRemove={popup.storedRepositoryToRemove}
2767+
isDeletingCurrentWorktree={popup.isDeletingCurrentWorktree}
27662768
dispatcher={this.props.dispatcher}
27672769
onDismissed={onPopupDismissedFn}
27682770
/>

app/src/ui/repositories-list/repositories-list.tsx

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -449,6 +449,10 @@ export class RepositoriesList extends React.Component<
449449
: item.repository instanceof Repository
450450
? item.repository
451451
: null
452+
const storedRepositoryToRemove =
453+
item.repository instanceof Repository && !item.isVirtualLinkedWorktree
454+
? item.repository
455+
: undefined
452456

453457
if (repository === null) {
454458
return
@@ -458,6 +462,11 @@ export class RepositoriesList extends React.Component<
458462
type: PopupType.DeleteWorktree,
459463
repository,
460464
worktreePath,
465+
storedRepositoryToRemove,
466+
isDeletingCurrentWorktree:
467+
this.props.selectedRepository !== null &&
468+
normalizePath(this.props.selectedRepository.path) ===
469+
normalizePath(worktreePath),
461470
})
462471
}
463472

@@ -490,7 +499,11 @@ export class RepositoriesList extends React.Component<
490499
onOpenInExternalEditor: this.props.onOpenInExternalEditor,
491500
askForConfirmationOnRemoveRepository:
492501
this.props.askForConfirmationOnRemoveRepository,
493-
isLinkedWorktreeRow: item.isVirtualLinkedWorktree,
502+
isLinkedWorktreeRow:
503+
item.isVirtualLinkedWorktree ||
504+
(item.repository instanceof Repository &&
505+
item.repository.isLinkedWorktree),
506+
isVirtualLinkedWorktreeRow: item.isVirtualLinkedWorktree,
494507
isPrunableWorktreeRow: item.isPrunableWorktree,
495508
externalEditorLabel: this.props.externalEditorLabel,
496509
onChangeRepositoryAlias: this.onChangeRepositoryAlias,

app/src/ui/repositories-list/repository-list-item-context-menu.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ interface IRepositoryListItemContextMenuConfig {
1919
externalEditorLabel: string | undefined
2020
askForConfirmationOnRemoveRepository: boolean
2121
readonly isLinkedWorktreeRow?: boolean
22+
readonly isVirtualLinkedWorktreeRow?: boolean
2223
readonly isPrunableWorktreeRow?: boolean
2324
onViewInBrowser: (repository: Repositoryish) => void
2425
onOpenInNewWindow?: (repository: Repositoryish) => void
@@ -159,7 +160,10 @@ const buildAliasMenuItems = (
159160
): ReadonlyArray<IMenuItem> => {
160161
const { repository } = config
161162

162-
if (!(repository instanceof Repository) || config.isLinkedWorktreeRow) {
163+
if (
164+
!(repository instanceof Repository) ||
165+
config.isVirtualLinkedWorktreeRow
166+
) {
163167
return []
164168
}
165169

@@ -186,7 +190,10 @@ const buildGroupNameMenuItems = (
186190
): ReadonlyArray<IMenuItem> => {
187191
const { repository } = config
188192

189-
if (!(repository instanceof Repository) || config.isLinkedWorktreeRow) {
193+
if (
194+
!(repository instanceof Repository) ||
195+
config.isVirtualLinkedWorktreeRow
196+
) {
190197
return []
191198
}
192199

app/src/ui/repositories-list/worktree-list-items.ts

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,22 @@ export const getDisplayTitle = (repository: Repositoryish) =>
2323
? repository.alias
2424
: repository.name
2525

26+
const getLinkedWorktreeDisplayTitle = (
27+
repository: Repositoryish,
28+
worktreePath?: string
29+
) =>
30+
repository instanceof Repository && repository.alias != null
31+
? repository.alias
32+
: Path.basename(worktreePath ?? repository.path)
33+
2634
export const getRepositoryListTitle = (
2735
repository: Repositoryish,
2836
showWorktreesInSidebar: boolean
2937
) =>
3038
showWorktreesInSidebar &&
3139
repository instanceof Repository &&
3240
repository.isLinkedWorktree
33-
? Path.basename(repository.path)
41+
? getLinkedWorktreeDisplayTitle(repository)
3442
: getDisplayTitle(repository)
3543

3644
const getVirtualRepositoryId = (worktreePath: string) => {
@@ -138,7 +146,7 @@ export function toSortedRepositoryListItems({
138146
: null
139147
const title =
140148
isLinkedWorktree || isVirtualLinkedWorktree
141-
? Path.basename(worktreePath)
149+
? getLinkedWorktreeDisplayTitle(repository, worktreePath)
142150
: getDisplayTitle(repository)
143151
const defaultBranchName =
144152
repoState?.defaultBranchName ??
@@ -313,12 +321,6 @@ export function toSortedRepositoryListItems({
313321

314322
for (const repository of orphanLinkedRepos) {
315323
items.push(toListItem(repository, false))
316-
appendVirtualWorktreeItems(
317-
items,
318-
repository,
319-
repository,
320-
emittedVirtualPaths
321-
)
322324
}
323325

324326
return items

app/src/ui/worktrees/delete-worktree-dialog.tsx

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ import {
1616
interface IDeleteWorktreeDialogProps {
1717
readonly repository: Repository
1818
readonly worktreePath: string
19+
readonly storedRepositoryToRemove?: Repository
20+
readonly isDeletingCurrentWorktree?: boolean
1921
readonly dispatcher: Dispatcher
2022
readonly onDismissed: () => void
2123
}
@@ -66,9 +68,13 @@ export class DeleteWorktreeDialog extends React.Component<
6668
private onDeleteWorktree = async () => {
6769
this.setState({ isDeleting: true })
6870

69-
const { repository, worktreePath, dispatcher } = this.props
70-
const isDeletingCurrentWorktree =
71-
normalizePath(repository.path) === normalizePath(worktreePath)
71+
const {
72+
repository,
73+
worktreePath,
74+
dispatcher,
75+
storedRepositoryToRemove,
76+
isDeletingCurrentWorktree = false,
77+
} = this.props
7278

7379
const mainPathForCleanup = await getMainWorktreePath(repository)
7480

@@ -94,9 +100,13 @@ export class DeleteWorktreeDialog extends React.Component<
94100
const mainRepo = addedRepos[0]
95101
await dispatcher.selectRepository(mainRepo)
96102
await removeWorktree(mainRepo, worktreePath)
97-
await dispatcher.removeRepository(repository, false)
98103
} else {
99104
await removeWorktree(repository, worktreePath)
105+
}
106+
107+
if (storedRepositoryToRemove !== undefined) {
108+
await dispatcher.removeRepository(storedRepositoryToRemove, false)
109+
} else if (!isDeletingCurrentWorktree) {
100110
await dispatcher.refreshRepository(repository)
101111
}
102112
} catch (e) {

app/test/unit/repositories-list-grouping-test.ts

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -390,4 +390,92 @@ describe('repository list grouping', () => {
390390
await rm(tempRoot, { recursive: true, force: true })
391391
}
392392
})
393+
394+
it('does not synthesize linked worktree siblings under orphan linked worktrees', async () => {
395+
const tempRoot = await mkdtemp(
396+
path.join(os.tmpdir(), 'github-desktop-plus-worktree-orphan-leaf-')
397+
)
398+
try {
399+
const mainRepoPath = path.join(tempRoot, 'repo')
400+
const linkedRepoPath = path.join(tempRoot, 'repo-feature-a')
401+
const secondLinkedRepoPath = path.join(tempRoot, 'repo-feature-b')
402+
403+
await mkdir(path.join(mainRepoPath, '.git'), { recursive: true })
404+
await mkdir(path.join(mainRepoPath, '.git', 'worktrees', 'feature-a'), {
405+
recursive: true,
406+
})
407+
await mkdir(path.join(mainRepoPath, '.git', 'worktrees', 'feature-b'), {
408+
recursive: true,
409+
})
410+
await mkdir(linkedRepoPath, { recursive: true })
411+
await writeFile(
412+
path.join(linkedRepoPath, '.git'),
413+
'gitdir: ../repo/.git/worktrees/feature-a\n'
414+
)
415+
await writeFile(
416+
path.join(mainRepoPath, '.git', 'worktrees', 'feature-a', 'commondir'),
417+
'../..\n'
418+
)
419+
await writeFile(
420+
path.join(mainRepoPath, '.git', 'worktrees', 'feature-b', 'commondir'),
421+
'../..\n'
422+
)
423+
424+
const linkedRepo = new Repository(
425+
linkedRepoPath,
426+
20,
427+
gitHubRepoFixture({ owner: 'example', name: 'repo' }),
428+
false,
429+
'custom alias'
430+
)
431+
432+
cache.set(linkedRepo.id, {
433+
aheadBehind: null,
434+
changedFilesCount: 0,
435+
branchName: 'feature/a',
436+
defaultBranchName: 'main',
437+
allWorktrees: [
438+
{
439+
path: mainRepoPath,
440+
head: 'a',
441+
branch: 'refs/heads/main',
442+
isDetached: false,
443+
type: 'main',
444+
isLocked: false,
445+
isPrunable: false,
446+
},
447+
{
448+
path: linkedRepoPath,
449+
head: 'b',
450+
branch: 'refs/heads/feature/a',
451+
isDetached: false,
452+
type: 'linked',
453+
isLocked: false,
454+
isPrunable: false,
455+
},
456+
{
457+
path: secondLinkedRepoPath,
458+
head: 'c',
459+
branch: 'refs/heads/feature/b',
460+
isDetached: false,
461+
type: 'linked',
462+
isLocked: false,
463+
isPrunable: false,
464+
},
465+
],
466+
})
467+
468+
const grouped = groupRepositories([linkedRepo], cache, [], {
469+
showWorktreesInSidebar: true,
470+
})
471+
472+
assert.equal(grouped.length, 1)
473+
assert.equal(grouped[0].items.length, 1)
474+
assert.equal(grouped[0].items[0].repository.path, linkedRepoPath)
475+
assert.equal(grouped[0].items[0].isNestedWorktree, false)
476+
assert.equal(grouped[0].items[0].title, 'custom alias')
477+
} finally {
478+
await rm(tempRoot, { recursive: true, force: true })
479+
}
480+
})
393481
})

app/test/unit/repository-list-item-context-menu-test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ describe('repository list item context menu', () => {
6666
const items = generateRepositoryListContextMenu(
6767
buildConfig({
6868
isLinkedWorktreeRow: true,
69+
isVirtualLinkedWorktreeRow: true,
6970
onRemoveRepository: () => {
7071
removedRepository = true
7172
},
@@ -98,6 +99,30 @@ describe('repository list item context menu', () => {
9899
assert.equal(removedRepository, false)
99100
})
100101

102+
it('keeps alias and group name actions for saved linked worktree rows', () => {
103+
const items = generateRepositoryListContextMenu(
104+
buildConfig({
105+
isLinkedWorktreeRow: true,
106+
isVirtualLinkedWorktreeRow: false,
107+
onRemoveLinkedWorktree: () => {},
108+
})
109+
)
110+
const labels = items.flatMap(item => ('label' in item ? [item.label] : []))
111+
112+
assert(labels.includes('Change alias') || labels.includes('Change Alias'))
113+
assert(labels.includes('Remove alias') || labels.includes('Remove Alias'))
114+
assert(
115+
labels.includes('Change group name') ||
116+
labels.includes('Change Group Name')
117+
)
118+
assert(
119+
labels.includes('Restore group name') ||
120+
labels.includes('Restore Group Name')
121+
)
122+
assert(labels.includes('Delete…'))
123+
assert(!labels.includes('Remove…'))
124+
})
125+
101126
it('shows a prune action for stale worktree rows and keeps remove semantics', () => {
102127
let prunedStaleWorktrees = false
103128
let removedRepository = false
@@ -140,6 +165,7 @@ describe('repository list item context menu', () => {
140165
const items = generateRepositoryListContextMenu(
141166
buildConfig({
142167
isLinkedWorktreeRow: true,
168+
isVirtualLinkedWorktreeRow: true,
143169
isPrunableWorktreeRow: true,
144170
onPruneStaleWorktrees: () => {},
145171
})

0 commit comments

Comments
 (0)