From efbf676c30d38946857a8783b006484d1dc09f2c Mon Sep 17 00:00:00 2001 From: Raman Gupta Date: Tue, 20 Oct 2026 09:49:01 -0400 Subject: [PATCH] feat(branches): delete a merged branch with its remote branch in one go Cleaning up after a merged PR took a delete in the local list and another in the remote list. A merged local branch whose upstream still exists, holds nothing the branch lacks and isn't a shared integration branch now offers "Delete both". The worktree card's "Remove + branch" gets the same reach. When the remote branch can go too, it reads "Remove + branch + origin", and the dialog has a ticked "Also delete origin/..." box to keep it instead. The action is gone from the table's worktree sub-row: deleting a branch is a call to make with the card open, not from a hover lane. Both remote deletes are leased on the tip last fetched, so a branch someone pushed to since is refused rather than deleted. The remote goes first, leaving the local branch to retry from if the remote refuses. The table's name column now takes 30% of the table width (262-560dp) instead of a fixed 262dp, so long claude/... worktree branch names fit on a wide window. The sub-row's badges also line up with the repo row's now; they sat 16dp to the left before. Signed-off-by: Raman Gupta --- .../kotlin/com/gitvantage/app/AppState.kt | 40 +++++++++- .../kotlin/com/gitvantage/git/BranchOps.kt | 50 ++++++++++++ .../kotlin/com/gitvantage/git/WorktreeOps.kt | 32 ++++++-- .../kotlin/com/gitvantage/model/Worktrees.kt | 3 + .../kotlin/com/gitvantage/ui/DetailPanel.kt | 21 +++++ src/main/kotlin/com/gitvantage/ui/Popups.kt | 25 ++++++ src/main/kotlin/com/gitvantage/ui/RepoList.kt | 36 +++++++-- .../kotlin/com/gitvantage/ui/Worktrees.kt | 49 ++++++++++-- .../com/gitvantage/git/BranchOpsTest.kt | 38 +++++++++ .../com/gitvantage/git/WorktreeOpsTest.kt | 78 ++++++++++++++++++- 10 files changed, 346 insertions(+), 26 deletions(-) diff --git a/src/main/kotlin/com/gitvantage/app/AppState.kt b/src/main/kotlin/com/gitvantage/app/AppState.kt index 54604e8..49caf75 100644 --- a/src/main/kotlin/com/gitvantage/app/AppState.kt +++ b/src/main/kotlin/com/gitvantage/app/AppState.kt @@ -116,6 +116,23 @@ sealed interface Popup { data class Settings(val section: SettingsSection = SettingsSection.APPEARANCE) : Popup { override val ids: Set = emptySet() } + + /** + * A [Confirm] with one checkbox, for a destructive action with an optional extra reach — the + * worktree "Remove + branch" that can also delete the remote branch. The box is in the dialog, + * not a second button on the row, so the choice is stated where the consequences are. + */ + data class ConfirmWithOption( + val title: String, + val message: String, + val option: String, + val optionDefault: Boolean, + val confirmLabel: (Boolean) -> String, + val danger: Boolean, + val onConfirm: (Boolean) -> Unit, + ) : Popup { + override val ids: Set = emptySet() + } data class Confirm( val title: String, val message: String, @@ -2019,9 +2036,10 @@ class AppState(private val scope: CoroutineScope) { * * [removeBranch] also deletes the branch it held — offered only for an agent worktree whose * branch has already landed or never held anything, where the branch is leftover bookkeeping - * rather than work. + * rather than work. [removeUpstream] takes its remote branch with it too, when + * [Worktree.deletableUpstream] says that's safe. */ - fun removeWorktree(id: String, wt: Worktree, removeBranch: Boolean = false) { + fun removeWorktree(id: String, wt: Worktree, removeBranch: Boolean = false, removeUpstream: Boolean = false) { if (worktreesBusy) return worktreesBusy = true scope.launch { @@ -2029,6 +2047,7 @@ class AppState(private val scope: CoroutineScope) { WorktreeOps.remove( id, wt.path, force = wt.dirtyCount > 0, locked = wt.locked, alsoBranch = wt.ownBranch?.takeIf { removeBranch }, + alsoUpstream = wt.deletableUpstream?.takeIf { removeBranch && removeUpstream }, ) } toast(r.message) @@ -2282,6 +2301,23 @@ class AppState(private val scope: CoroutineScope) { } } + /** + * Delete a merged branch together with its upstream (see [BranchOps.deleteWithUpstream]). + * Shares [deletingRemoteBranch] with the remote list's delete: it's the same network round + * trip, and both rows for the pair then say "Deleting…". + */ + fun deleteBranchWithUpstream(id: String, name: String, upstream: String) { + if (deletingRemoteBranch != null) return + deletingRemoteBranch = upstream + scope.launch { + val r = withContext(Dispatchers.IO) { BranchOps.deleteWithUpstream(id, name, upstream) } + toast(r.message) + reloadBranches(id) + rescanRepos(listOf(id), fetch = false) + deletingRemoteBranch = null + } + } + /** Open the diff viewer for a repo and load its working-tree changes off the UI thread. */ fun openDiff(id: String) { diffRepoName = File(id).name diff --git a/src/main/kotlin/com/gitvantage/git/BranchOps.kt b/src/main/kotlin/com/gitvantage/git/BranchOps.kt index 0045457..c82502c 100644 --- a/src/main/kotlin/com/gitvantage/git/BranchOps.kt +++ b/src/main/kotlin/com/gitvantage/git/BranchOps.kt @@ -216,6 +216,56 @@ object BranchOps { else OpResult(repoPath, false, res.firstError("remote delete failed")) } + /** + * Delete a local branch *and* the remote branch it tracks, for a branch whose work has landed + * — the clean-up after a merged PR, which otherwise takes two deletes in two lists. + * + * The remote goes first, because it's the half that can fail (network, permissions, a + * protected branch): if it does, the local branch is left alone and the whole thing can just be + * tried again. Deleting local first would strand the remote branch with nothing on this side + * left to offer the retry from. + * + * The remote delete carries a lease on the tip we last fetched. The caller only offers this + * when that tip had nothing the local branch lacks, but the fetch may be old — if someone has + * pushed to the branch since, git refuses the delete instead of throwing their commits away. + */ + suspend fun deleteWithUpstream(repoPath: String, name: String, upstream: String): OpResult = + withContext(Dispatchers.IO) { + val dir = File(repoPath) + val pushed = deleteUpstreamLeased(dir, upstream) + if (!pushed.ok) return@withContext OpResult(repoPath, false, pushed.firstError("remote delete failed")) + val local = Git.run(dir, listOf("branch", "-D", name)) + if (local.ok) OpResult(repoPath, true, "Deleted $name and $upstream") + else OpResult(repoPath, false, "Deleted $upstream, but not $name: ${local.firstError("delete failed")}") + } + + /** + * `git push --delete refs/heads/` for [upstream] (e.g. "origin/foo"), leased on + * the tip last fetched into `refs/remotes/`: a branch someone pushed to since is + * refused rather than deleted. Shared by [deleteWithUpstream] and the worktree "Remove + branch". + */ + internal fun deleteUpstreamLeased(dir: File, upstream: String): Git.Result { + val ref = "refs/heads/${upstream.substringAfter('/')}" + val seen = Git.read(dir, "rev-parse", "--verify", "-q", "refs/remotes/$upstream").trim() + val lease = if (seen.isNotEmpty()) "--force-with-lease=$ref:$seen" else "--force-with-lease" + return Git.run(dir, listOf("push", lease, upstream.substringBefore('/'), "--delete", ref), Git.NETWORK_TIMEOUT) + } + + /** + * The upstream of local [branch] when deleting it on the remote loses nothing *beyond* the local + * branch: it still exists, has no commits [branch] lacks (so if [branch] is merged, so is it), + * and isn't a shared integration branch. Null otherwise, and for a branch with no upstream. + */ + internal fun deletableUpstream(dir: File, branch: String): String? { + val (upstream, track) = Git.read( + dir, "for-each-ref", "--format=%(upstream:short)$SEP%(upstream:track)", "refs/heads/$branch", + ).trim().split(SEP).let { it.getOrElse(0) { "" } to it.getOrElse(1) { "" } } + if (upstream.isEmpty() || !upstream.contains('/')) return null + val (_, behind, gone) = parseTrack(track) + if (gone || behind > 0 || Meta.isIntegrationBranch(upstream.substringAfter('/'))) return null + return upstream + } + /** Parse git's `%(upstream:track)` — "[ahead 2, behind 1]", "[gone]", or "". */ private fun parseTrack(track: String): Triple { if ("gone" in track) return Triple(0, 0, true) diff --git a/src/main/kotlin/com/gitvantage/git/WorktreeOps.kt b/src/main/kotlin/com/gitvantage/git/WorktreeOps.kt index 7912a35..06b4647 100644 --- a/src/main/kotlin/com/gitvantage/git/WorktreeOps.kt +++ b/src/main/kotlin/com/gitvantage/git/WorktreeOps.kt @@ -27,9 +27,8 @@ import kotlinx.coroutines.withContext * * Everything is read on the scan path because the *list* views show it now, not only the panel: * a worktree is a sub-row under its repo, and that row's badges are "3 modified", "↑2 vs main", - * "merged", its right-hand column is "2 hours ago · Ada Okafor", and whether its branch has landed - * is what decides if "Remove + branch" is offered at all. A lazily-loaded field would mean a row - * that silently omits a badge and hides an action until something else happened to open the pane. + * "merged", and its right-hand column is "2 hours ago · Ada Okafor". A lazily-loaded field would + * mean a row that silently omits a badge until something else happened to open the pane. * * That makes a repo with linked worktrees about twice the scan it used to be. It buys the four * questions the row asks, it is bounded by the number of worktrees the repo actually has, and it is @@ -170,6 +169,7 @@ object WorktreeOps { branchMerged = merged, branchEmpty = contained && !merged, tipBranch = tipBranch, + deletableUpstream = if (merged) BranchOps.deletableUpstream(dir, own!!) else null, ) } } @@ -263,6 +263,11 @@ object WorktreeOps { * only offers this for a branch it has already seen land, and if that changed underneath us * the branch survives and the message says so. Removal itself still counts as success — the * folder is gone either way, and reporting failure would invite a retry that can't help. + * + * [alsoUpstream] (only alongside [alsoBranch]) deletes the branch's remote counterpart as well, + * leased the way [BranchOps.deleteUpstreamLeased] describes. It goes before the local branch so + * the local `-d` still has the tracking ref around to be checked against. If the remote refuses, + * the rest still happens: the remote branch stays in the remote list, deletable from there. */ suspend fun remove( runFrom: String, @@ -270,6 +275,7 @@ object WorktreeOps { force: Boolean, locked: Boolean, alsoBranch: String? = null, + alsoUpstream: String? = null, ): OpResult = withContext(Dispatchers.IO) { val dir = File(runFrom) val args = buildList { @@ -284,12 +290,22 @@ object WorktreeOps { val name = File(path).name tidyAgentParent(path) if (alsoBranch == null) return@withContext OpResult(runFrom, true, "Removed worktree $name") + val upstreamRes = alsoUpstream?.let { BranchOps.deleteUpstreamLeased(dir, it) } val branchRes = Git.run(dir, listOf("branch", "-d", alsoBranch)) - OpResult( - runFrom, true, - if (branchRes.ok) "Removed worktree $name and branch $alsoBranch" - else "Removed worktree $name — branch kept: ${branchRes.firstError("delete failed")}", - ) + val removed = buildList { + add("worktree $name") + if (branchRes.ok) add("branch $alsoBranch") + if (upstreamRes?.ok == true) add(alsoUpstream) + } + val kept = buildList { + if (!branchRes.ok) add("branch kept: ${branchRes.firstError("delete failed")}") + if (upstreamRes?.ok == false) add("$alsoUpstream kept: ${upstreamRes.firstError("remote delete failed")}") + } + val said = "Removed " + when (removed.size) { + 1 -> removed[0] + else -> removed.dropLast(1).joinToString(", ") + " and " + removed.last() + } + OpResult(runFrom, true, if (kept.isEmpty()) said else "$said — ${kept.joinToString("; ")}") } /** diff --git a/src/main/kotlin/com/gitvantage/model/Worktrees.kt b/src/main/kotlin/com/gitvantage/model/Worktrees.kt index fbc399c..c166568 100644 --- a/src/main/kotlin/com/gitvantage/model/Worktrees.kt +++ b/src/main/kotlin/com/gitvantage/model/Worktrees.kt @@ -29,6 +29,9 @@ data class Worktree( // Sessions detach their worktree once the work is pushed, which leaves the tree and its branch // unconnected as far as git is concerned while being one piece of work as far as anyone else is. val tipBranch: String? = null, + // The remote branch [ownBranch] tracks, when [branchMerged] and deleting it too loses nothing — + // see BranchOps.deletableUpstream. What lets "Remove + branch" offer to clean up the remote too. + val deletableUpstream: String? = null, val lastRelative: String = "", // git's relative date for the worktree's HEAD commit val lastAuthor: String = "", // who made that commit // Epoch *seconds* of that commit, as git reports them. Carried alongside the relative string diff --git a/src/main/kotlin/com/gitvantage/ui/DetailPanel.kt b/src/main/kotlin/com/gitvantage/ui/DetailPanel.kt index 2b35aa6..6640cc7 100644 --- a/src/main/kotlin/com/gitvantage/ui/DetailPanel.kt +++ b/src/main/kotlin/com/gitvantage/ui/DetailPanel.kt @@ -1156,6 +1156,14 @@ private fun BranchRow( else -> null } val pushing = state.pushingBranch == b.name + // The upstream to take down with the branch, when that loses nothing: the branch is merged, and + // its upstream has no commits the branch lacks — so the upstream is merged too. Off for the + // shared branches the remote list won't delete either, and for an upstream already gone. + val deletableUpstream = b.upstream?.takeIf { + canDelete && hasRemote && b.merged && !b.upstreamGone && b.upstreamBehind == 0 && + !Meta.isIntegrationBranch(it.substringAfter('/')) + } + val deletingUpstream = deletableUpstream != null && state.deletingRemoteBranch == deletableUpstream HoverRow { hovered -> // Line 1: branch name · mainline-relationship badge · ahead-of-mainline Row(verticalAlignment = Alignment.CenterVertically, horizontalArrangement = Arrangement.spacedBy(8.dp)) { @@ -1288,6 +1296,19 @@ private fun BranchRow( ) { state.deleteBranch(id, b.name, force = true) } } } + // The after-a-merged-PR clean-up in one click rather than a delete in each list. + // No prompt, for the same reason neither half has one on its own: both are merged. + if (deletableUpstream != null) { + HoverTip( + "Deletes “${b.name}” here and “$deletableUpstream” on the remote. Both are " + + "already in mainline, so nothing is lost — and if someone has pushed to " + + "it since your last fetch, the remote refuses and both stay.", + ) { + RowAction(if (deletingUpstream) "Deleting…" else "Delete both", danger = true) { + if (!deletingUpstream) state.deleteBranchWithUpstream(id, b.name, deletableUpstream) + } + } + } } } } diff --git a/src/main/kotlin/com/gitvantage/ui/Popups.kt b/src/main/kotlin/com/gitvantage/ui/Popups.kt index d0dc9bb..af0da3f 100644 --- a/src/main/kotlin/com/gitvantage/ui/Popups.kt +++ b/src/main/kotlin/com/gitvantage/ui/Popups.kt @@ -89,6 +89,7 @@ fun PopupHost(state: AppState, popup: Popup) { is Popup.Commit -> CommitPopup(state, popup.id) is Popup.CommitWorktree -> CommitWorktreePopup(state, popup) is Popup.Confirm -> ConfirmPopup(state, popup) + is Popup.ConfirmWithOption -> ConfirmWithOptionPopup(state, popup) is Popup.Settings -> SettingsPopup(state, popup.section) } } @@ -684,6 +685,30 @@ private fun ConfirmPopup(state: AppState, p: Popup.Confirm) { } } +@Composable +private fun ConfirmWithOptionPopup(state: AppState, p: Popup.ConfirmWithOption) { + var checked by remember(p) { mutableStateOf(p.optionDefault) } + Modal({ state.popup = null }) { + ModalHeader(p.title, p.message) + Column(Modifier.padding(18.dp), verticalArrangement = Arrangement.spacedBy(12.dp)) { + Row( + Modifier.onTap { checked = !checked }, + verticalAlignment = Alignment.CenterVertically, + horizontalArrangement = Arrangement.spacedBy(8.dp), + ) { + SelectBox(checked = checked) + // Wraps rather than truncating: the option names a ref, and agent branch names are + // long enough that one line would cut off the half saying where it's deleted. + Txt(p.option, 12.sp, Tokens.text2, maxLines = 3, modifier = Modifier.weight(1f)) + } + ModalButtons({ state.popup = null }, p.confirmLabel(checked), true, p.danger) { + state.popup = null + p.onConfirm(checked) + } + } + } +} + // ---------- toast ---------- /** Transient op-result pill, bottom-center. The empty area is click-through. */ diff --git a/src/main/kotlin/com/gitvantage/ui/RepoList.kt b/src/main/kotlin/com/gitvantage/ui/RepoList.kt index e5f9f16..18d20f2 100644 --- a/src/main/kotlin/com/gitvantage/ui/RepoList.kt +++ b/src/main/kotlin/com/gitvantage/ui/RepoList.kt @@ -7,6 +7,7 @@ import androidx.compose.foundation.background import androidx.compose.foundation.border import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.BoxWithConstraints import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.Spacer @@ -31,6 +32,7 @@ import androidx.compose.ui.graphics.Color import androidx.compose.ui.input.pointer.PointerIcon import androidx.compose.ui.input.pointer.pointerHoverIcon import androidx.compose.ui.text.font.FontWeight +import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.sp import com.gitvantage.app.AppState @@ -113,21 +115,26 @@ private fun EmptyState(registryEmpty: Boolean, onAdd: () -> Unit) { @OptIn(androidx.compose.foundation.ExperimentalFoundationApi::class) @Composable -private fun TableView(state: AppState, groups: List) { +private fun TableView(state: AppState, groups: List) = BoxWithConstraints(Modifier.fillMaxSize()) { + // The name column's width, decided once for the whole table so a repo row and its worktree + // sub-rows keep one column edge between them. A share of the table rather than a fixed width: + // on a wide window the names — long `claude/…` branch names especially — are what's worth the + // room, and the badge and last-commit columns already had more slack than they could use. + val nameWidth = (maxWidth * NAME_SHARE).coerceIn(NAME_MIN, NAME_MAX) LazyColumn(Modifier.fillMaxSize()) { groups.forEach { group -> if (group.showHeader) { stickyHeader(key = "h-${group.nsLabel}-${group.name}") { GroupHeaderTable(group) } } group.repos.forEach { rv -> - item(key = "${group.name}-${rv.id}") { RepoRow(state, rv) } + item(key = "${group.name}-${rv.id}") { RepoRow(state, rv, nameWidth) } // Sub-rows are separate list items rather than a Column inside the parent's row, so // a repo with a dozen worktrees doesn't build a dozen rows' worth of layout to // scroll past off-screen — the same reason the parents are items and not one Column. if (state.worktreeRowsOpen(rv.id)) { rv.worktrees.forEach { wtv -> item(key = "${group.name}-${rv.id}-${wtv.path}") { - WorktreeSubRow(state, rv, wtv, barWidth(state)) + WorktreeSubRow(state, rv, wtv, barWidth(state), nameWidth - WORKTREE_INDENT) } } } @@ -136,6 +143,19 @@ private fun TableView(state: AppState, groups: List) { } } +/** The table's name column: [NAME_SHARE] of its width, within these bounds. */ +private const val NAME_SHARE = 0.3f +private val NAME_MIN = 262.dp +private val NAME_MAX = 560.dp + +/** + * How much narrower a worktree sub-row's name column is than its repo's: the sub-row's name starts + * further right, and this is what's left of that once the two rows' leading pieces are summed, so + * the badge columns start on one edge. Measured on screen, at 1× — it was 24 when both widths were + * fixed, which left the sub-row's badges 16dp left of the repo's. + */ +private val WORKTREE_INDENT = 8.dp + /** The row accent bar's width, which the sub-rows continue. */ private fun barWidth(state: AppState) = if (state.emphasis == Emphasis.SUBTLE) 2f else 3f @@ -156,7 +176,7 @@ private fun GroupHeaderTable(group: RepoGroup) { @OptIn(androidx.compose.foundation.layout.ExperimentalLayoutApi::class) @Composable -private fun RepoRow(state: AppState, rv: RepoView) { +private fun RepoRow(state: AppState, rv: RepoView, nameWidth: Dp) { val selected = rv.id == state.selectedId val checked = rv.id in state.bulkSelected val rowBg = when { @@ -184,11 +204,11 @@ private fun RepoRow(state: AppState, rv: RepoView) { Box(Modifier.width(12.dp), contentAlignment = Alignment.CenterStart) { StatusDot(rv.accent, 9) } - // Name stays tight on the left; the badges and last-commit columns share the row's slack by - // weight, so on wide windows the space sits as one balanced center gap rather than a single - // huge empty box — and the last-commit column is wide enough to never truncate. + // Name takes its share of the table (see TableView); the badges and last-commit columns + // split what's left by weight, so the remaining space sits as one balanced center gap rather + // than a single huge empty box — and the last-commit column is wide enough to never truncate. // Col 2 — name / branch / tags - Column(Modifier.width(262.dp), verticalArrangement = Arrangement.spacedBy(2.dp)) { + Column(Modifier.width(nameWidth), verticalArrangement = Arrangement.spacedBy(2.dp)) { Row(verticalAlignment = Alignment.CenterVertically, horizontalArrangement = Arrangement.spacedBy(6.dp)) { Txt(rv.repo.name, 13.5.sp, Tokens.text, FontWeight.Bold, modifier = Modifier.weight(1f, fill = false)) // Only where there is something to disclose. A chevron on every repo would promise a diff --git a/src/main/kotlin/com/gitvantage/ui/Worktrees.kt b/src/main/kotlin/com/gitvantage/ui/Worktrees.kt index c36ce1b..db18544 100644 --- a/src/main/kotlin/com/gitvantage/ui/Worktrees.kt +++ b/src/main/kotlin/com/gitvantage/ui/Worktrees.kt @@ -37,6 +37,7 @@ import androidx.compose.ui.input.pointer.pointerHoverIcon import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.unit.IntOffset +import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.sp import androidx.compose.ui.window.Popup @@ -84,7 +85,7 @@ import com.gitvantage.model.WorktreeChange * remote. */ @Composable -fun WorktreeSubRow(state: AppState, rv: RepoView, wtv: WorktreeView, barWidth: Float) { +fun WorktreeSubRow(state: AppState, rv: RepoView, wtv: WorktreeView, barWidth: Float, nameWidth: Dp) { val wt = wtv.wt val source = remember { MutableInteractionSource() } val hovered by source.collectIsHoveredAsState() @@ -103,7 +104,7 @@ fun WorktreeSubRow(state: AppState, rv: RepoView, wtv: WorktreeView, barWidth: F StatusDot(wtv.accent, 8) // Identity: the branch is the name, the folder is the footnote — the inverse of the old // worktree row, where a generated slug like "fix-rounding-9f3ab2" was the headline. - Column(Modifier.width(238.dp), verticalArrangement = Arrangement.spacedBy(1.dp)) { + Column(Modifier.width(nameWidth), verticalArrangement = Arrangement.spacedBy(1.dp)) { Txt("⑂ ${wtv.label}", 12.5.sp, Tokens.text, FontWeight.SemiBold, font = MonoFont) PathTip(wt.path) { Txt(shortPath(wt.path), 10.5.sp, Tokens.muted2, font = MonoFont) } } @@ -125,9 +126,8 @@ fun WorktreeSubRow(state: AppState, rv: RepoView, wtv: WorktreeView, barWidth: F RowAction("Log") { state.openWorktreeLog(wt.path) } RowAction("Terminal") { state.openTerminal(wt.path) } } - if (canRemoveWithBranch(wt)) { - RowAction("Remove + branch", danger = true) { confirmRemove(state, rv, wt, alsoBranch = true) } - } + // No "Remove + branch" here: deleting a branch — and maybe its remote — is a call to + // make with the card open, looking at the branch, not from a lane a pass-over reveals. } WorktreeSnoozeChip(rv, wtv) } @@ -359,7 +359,7 @@ private fun WorktreeCard(state: AppState, rv: RepoView, wtv: WorktreeView) { if (!state.worktreesBusy && !wt.missing) { RowAction("Remove", danger = true) { confirmRemove(state, rv, wt, alsoBranch = false) } if (canRemoveWithBranch(wt)) { - RowAction("Remove + branch", danger = true) { confirmRemove(state, rv, wt, alsoBranch = true) } + RemoveWithBranchAction(state, rv, wt) } } } @@ -627,8 +627,45 @@ private fun WorktreeSnoozeChip(rv: RepoView, wtv: WorktreeView, size: Int = 27) private fun canRemoveWithBranch(wt: Worktree) = wt.agent && (wt.branchMerged || wt.branchEmpty) && wt.ownBranch != null && !wt.missing +/** + * "Remove + branch", or "Remove + branch + origin" when the remote branch can go too — the remote + * named in the label, because reaching the server is the part that shouldn't be a surprise. The + * dialog still has the box to leave it out. + */ +@Composable +private fun RemoveWithBranchAction(state: AppState, rv: RepoView, wt: Worktree) { + val upstream = wt.deletableUpstream + HoverTip( + if (upstream != null) { + "Deletes this folder and the branch “${wt.ownBranch}” — and offers to delete " + + "“$upstream” on the remote too, since it's merged as well." + } else { + "Deletes this folder and the branch “${wt.ownBranch}”." + }, + ) { + RowAction(if (upstream != null) "Remove + branch + ${upstream.substringBefore('/')}" else "Remove + branch", danger = true) { + confirmRemove(state, rv, wt, alsoBranch = true) + } + } +} + private fun confirmRemove(state: AppState, rv: RepoView, wt: Worktree, alsoBranch: Boolean) { val branch = wt.ownBranch?.takeIf { alsoBranch } + // The branch has a remote counterpart that's merged too: offer to take it as well, ticked — + // after a merged PR that's what's wanted — but as a box in the dialog, because unlike the rest + // this reaches the server, and it should be seen being agreed to. + val upstream = wt.deletableUpstream?.takeIf { branch != null } + if (branch != null && upstream != null) { + state.popup = AppPopup.ConfirmWithOption( + "Remove worktree “$branch” and its branch?", + removeWorktreeDetail(wt, alsoBranch = branch), + "Also delete “$upstream” on ${upstream.substringBefore('/')} — it's merged too", + optionDefault = true, + confirmLabel = { if (it) "Remove all" else "Remove both" }, + danger = true, + ) { alsoUpstream -> state.removeWorktree(rv.id, wt, removeBranch = true, removeUpstream = alsoUpstream) } + return + } state.popup = AppPopup.Confirm( if (branch != null) "Remove worktree “$branch” and its branch?" else "Remove worktree “${wt.ownBranch ?: wt.name}”?", removeWorktreeDetail(wt, alsoBranch = branch), diff --git a/src/test/kotlin/com/gitvantage/git/BranchOpsTest.kt b/src/test/kotlin/com/gitvantage/git/BranchOpsTest.kt index f56f903..189b007 100644 --- a/src/test/kotlin/com/gitvantage/git/BranchOpsTest.kt +++ b/src/test/kotlin/com/gitvantage/git/BranchOpsTest.kt @@ -238,5 +238,43 @@ val BranchOperations by testSuite { assert(!result.message.startsWith("remote:")) assert(gitAllowingFailure(originOf(work), "rev-parse", "--verify", "-q", Sandbox.MAIN) == 0) } + + // --- deleteWithUpstream ------------------------------------------------------------- + + test("deleteWithUpstream removes both the local branch and its remote branch") { + val work = repo() + git(work, "switch", "-q", "-c", "feature") + commit(work, "f.txt", "mine\n", "feature work") + git(work, "push", "-q", "-u", "origin", "feature") + git(work, "switch", "-q", Sandbox.MAIN) + git(work, "merge", "-q", "--no-ff", "-m", "merge feature", "feature") + + val result = BranchOps.deleteWithUpstream(work.path, "feature", "origin/feature") + + assert(result.ok) + assert(result.message == "Deleted feature and origin/feature") + assert("feature" !in branches(work)) + assert(gitAllowingFailure(originOf(work), "rev-parse", "--verify", "-q", "feature") != 0) + assert(gitAllowingFailure(work, "rev-parse", "--verify", "-q", "refs/remotes/origin/feature") != 0) + } + + test("deleteWithUpstream keeps both when someone pushed to the remote since the last fetch") { + val work = repo() + git(work, "switch", "-q", "-c", "feature") + commit(work, "f.txt", "mine\n", "feature work") + git(work, "push", "-q", "-u", "origin", "feature") + git(work, "switch", "-q", Sandbox.MAIN) + val other = clone(originOf(work), "other") + git(other, "switch", "-q", "feature") + commit(other, "g.txt", "theirs\n", "their follow-up") + git(other, "push", "-q", "origin", "feature") + + val result = BranchOps.deleteWithUpstream(work.path, "feature", "origin/feature") + + assert(!result.ok) + // Their commit survives on the remote, and the local branch is still here to retry from. + assert(gitAllowingFailure(originOf(work), "rev-parse", "--verify", "-q", "feature") == 0) + assert("feature" in branches(work)) + } } } diff --git a/src/test/kotlin/com/gitvantage/git/WorktreeOpsTest.kt b/src/test/kotlin/com/gitvantage/git/WorktreeOpsTest.kt index cbc587f..76da079 100644 --- a/src/test/kotlin/com/gitvantage/git/WorktreeOpsTest.kt +++ b/src/test/kotlin/com/gitvantage/git/WorktreeOpsTest.kt @@ -140,8 +140,7 @@ val WorktreeOperations by testSuite { val side = WorktreeOps.listWithWork(work.path).single { !it.isCurrent } - // Gates the "merged" badge and whether "Remove + branch" is offered at all, both of which - // the table's sub-rows show — so it cannot be a detail-panel-only field. + // Gates the "merged" badge, which the table's sub-rows show — so it cannot be a detail-panel-only field. assert(side.branchMerged) assert(!side.branchEmpty) assert(!side.unlanded) @@ -246,5 +245,80 @@ val WorktreeOperations by testSuite { assert(result.message == "Pruned stale worktree entries") assert(WorktreeOps.list(work.path).size == 1) } + + // --- the remote branch alongside "Remove + branch" ---------------------------------- + + test("a merged worktree branch with a pushed upstream offers that upstream for deletion") { + val work = repo() + val tree = File(root, "wt") + git(work, "worktree", "add", "-q", "-b", "side", tree.path) + commit(tree, "f.txt", "landed\n", "work on side") + git(tree, "push", "-q", "-u", "origin", "side") + git(work, "merge", "-q", "--no-ff", "-m", "merge side", "side") + + val side = WorktreeOps.listWithWork(work.path).single { !it.isCurrent } + + assert(side.deletableUpstream == "origin/side") + } + + test("an upstream with commits the local branch lacks is not offered") { + val work = repo() + val tree = File(root, "wt") + git(work, "worktree", "add", "-q", "-b", "side", tree.path) + commit(tree, "f.txt", "landed\n", "work on side") + git(tree, "push", "-q", "-u", "origin", "side") + git(work, "merge", "-q", "--no-ff", "-m", "merge side", "side") + val other = clone(originOf(work), "other") + git(other, "switch", "-q", "side") + commit(other, "g.txt", "theirs\n", "their follow-up") + git(other, "push", "-q", "origin", "side") + git(work, "fetch", "-q") + + val side = WorktreeOps.listWithWork(work.path).single { !it.isCurrent } + + assert(side.branchMerged) + assert(side.deletableUpstream == null) + } + + test("remove with alsoUpstream deletes the worktree, its branch and the remote branch") { + val work = repo() + val tree = File(root, "wt") + git(work, "worktree", "add", "-q", "-b", "side", tree.path) + commit(tree, "f.txt", "landed\n", "work on side") + git(tree, "push", "-q", "-u", "origin", "side") + git(work, "merge", "-q", "--no-ff", "-m", "merge side", "side") + + val result = WorktreeOps.remove( + work.path, tree.path, force = false, locked = false, alsoBranch = "side", alsoUpstream = "origin/side", + ) + + assert(result.ok) + assert(result.message == "Removed worktree wt, branch side and origin/side") + assert(!tree.exists()) + assert("side" !in branches(work)) + assert(gitAllowingFailure(originOf(work), "rev-parse", "--verify", "-q", "side") != 0) + } + + test("remove keeps a remote branch someone pushed to since the last fetch, and says so") { + val work = repo() + val tree = File(root, "wt") + git(work, "worktree", "add", "-q", "-b", "side", tree.path) + commit(tree, "f.txt", "landed\n", "work on side") + git(tree, "push", "-q", "-u", "origin", "side") + git(work, "merge", "-q", "--no-ff", "-m", "merge side", "side") + val other = clone(originOf(work), "other") + git(other, "switch", "-q", "side") + commit(other, "g.txt", "theirs\n", "their follow-up") + git(other, "push", "-q", "origin", "side") + + val result = WorktreeOps.remove( + work.path, tree.path, force = false, locked = false, alsoBranch = "side", alsoUpstream = "origin/side", + ) + + assert(result.ok) + assert("origin/side kept" in result.message) + assert("side" !in branches(work)) + assert(gitAllowingFailure(originOf(work), "rev-parse", "--verify", "-q", "side") == 0) + } } }