Support reordering empty branches in single-branch mode#14669
Open
estib-vega wants to merge 8 commits into
Open
Support reordering empty branches in single-branch mode#14669estib-vega wants to merge 8 commits into
estib-vega wants to merge 8 commits into
Conversation
estib-vega
force-pushed
the
move-branch-reorder-tests
branch
from
19e4615 to
5ad2d8c
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Extends but_workspace::branch::move_branch to support reordering empty branches in single-branch (ad-hoc) workspaces by updating branch_order metadata, while keeping non-empty branch moves rejected (since they require a true rebase).
Changes:
- Split
move_branchinto workspace-kind-specific handlers, adding an ad-hoc/single-branch implementation path. - Implemented metadata-only reordering for empty branches in ad-hoc mode via
branch_orderpersistence. - Added positive/negative regression tests covering ad-hoc empty-branch reorder and rejection of non-empty moves.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| crates/but-workspace/src/branch/move_branch.rs | Adds ad-hoc handler and branch-order reorder helper; refactors control flow to dispatch by WorkspaceKind. |
| crates/but-workspace/tests/workspace/branch/move_branch.rs | Adds targeted tests validating ad-hoc empty-branch reorder and error behavior for non-empty branch moves. |
estib-vega
force-pushed
the
move-branch-reorder-tests
branch
2 times, most recently
from
88b462a to
f21b138
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f21b138aa9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
estib-vega
force-pushed
the
move-branch-reorder-tests
branch
from
e5e07f4 to
313136f
Compare
`move_branch` previously bailed on ad-hoc (single-branch) workspaces before ever touching metadata. Add a dedicated single-branch handler that reorders two empty branches by rewriting the `branch_order` metadata (mirroring `create_reference`'s ad-hoc handling), and split `move_branch` into a `match` on the workspace kind that dispatches to the ad-hoc and managed handlers. Non-empty branches in single-branch mode are still rejected, as moving the commit-owning branch needs a real rebase. Adds positive/negative tests for the single-branch path.
When neither the subject nor target branch is present in the persisted `branch_order` (stale/empty metadata), the reorder used to overwrite the stored order with just `[subject]`, dropping the rest of the stack ordering. Instead, add both branches in the right order (subject on top of target) without discarding any existing entries. Adds unit tests for the reorder helper covering all presence cases. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The backend supports reordering empty branches in single-branch mode, but the UI couldn't trigger it. `MoveBranchDzHandler.accepts` only allowed cross-stack moves of non-empty branches; relax it to also accept reordering empty branches within the same stack (single-branch mode), rejecting no-op self-drops. Reordering empty branches is metadata-only and doesn't move HEAD, so the `move_branch` mutation now also invalidates the Stacks/StackDetails tags to make the branch list refetch and reflect the new order. Adds a Playwright test that creates three empty dependent branches in single-branch mode, drags one to reorder within the stack, and asserts the new branch-header order. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
In single-branch (ad-hoc) mode a move can place the subject above the checked-out tip, which would drop it out of the projection until HEAD moves. Mirror `create_reference`: `move_branch::Outcome` now carries a `new_tip` (set when the subject moves on top of the checked-out branch), and the `move_branch` API checks it out after the metadata-only move. Adds Rust tests for the `new_tip` contract (returned when moving above the tip, unset otherwise) and a Playwright test that drags an empty branch above the checked-out ref and asserts the new tip is checked out. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Follow the repo-wide insta→snapbox migration: replace the remaining `insta::assert_snapshot!` calls in the single-branch-mode tests with `snapbox::assert_data_eq!` / `snapbox::str!`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`crate::branch::create_reference` is both a function and a module, so the bare intra-doc link was ambiguous and failed `cargo doc -D warnings`. Disambiguate to the function with trailing parentheses. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Dry-run safety (#2): move_branch no longer persists the reordered ad-hoc branch order itself; it returns it in the Outcome and the API persists it only for non-dry-run calls, so a dry-run preview never mutates branch order. Audit: branch create/remove have no dry-run mode, so only move_branch was affected. - Empty-onto-base (#3): only a non-empty *subject* needs a real rebase, so allow moving an empty branch onto a commit-owning target (e.g. the base) instead of bailing when the target owns commits. - Clobbering (#4): a movable subject is always tracked in branch_order, so the "neither ref tracked" path is unreachable (untracked refs aren't projected as movable segments and fail to be found first). Added a defensive entrypoint fallback and a regression test documenting this. - ondrop (#5): skip the redundant source-stack PR refresh on a same-stack reorder. Adds Rust tests for the returned-not-persisted order, empty-onto-base, the untracked-refs invariant, and updates the single-branch suite.
estib-vega
force-pushed
the
move-branch-reorder-tests
branch
from
313136f to
fe5c1a3
Compare
Transaction::stack_branch_on ignored the new_tip and branch_stack_order fields that move_branch's Outcome exposes for ad-hoc (single-branch) moves. Transactions operate on managed workspaces only and their RecordingMetadata can't persist branch stack order, so rather than silently drop a metadata reorder or a required checkout, bail loudly if either field is ever populated, and document the limitation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🧢 Changes
move_branchnow handles ad-hoc (single-branch) workspaces instead of bailing: it reorders two empty branches by rewriting thebranch_ordermetadata (mirroringcreate_reference's ad-hoc handling), and its body is split into amatchon the workspace kind that dispatches to dedicated single-branch and managed-workspace handlers. Non-empty branches in single-branch mode are still rejected, since moving the commit-owning branch needs a real rebase. Adds positive and negative tests for the single-branch path.☕️ Reasoning
Reordering empty branches in single-branch mode was untested and, it turned out, unsupported —
move_branchrejected all non-managed workspaces before ever touching branch-order metadata. This wires up the pure-metadata reorder path so single-branch users get the same reordering behavior as managed workspaces.