Make worktree creation checkbox-driven in the TUI#979
Conversation
|
Hi, Thanks a lot for your PR! Indeed it's opinionated, and while I am not against it, I am not entirely sure if it's the right move. Could you also add the same settings and checkbox to the web UI so we have parity there? |
Address three edge cases in the auto-derived worktree branch flow: - Latin-script titles (cafe, naive, Strasse) no longer silently lose characters; ascii_fold transliterates common diacritics and ligatures before sanitization. Unsupported scripts (CJK, emoji) still fall back to the "session" placeholder so users know to set an explicit name. - Auto-derived branch names now dedupe against existing git branches and active aoe sessions, suffixing -2, -3, ... when needed. Only applies when the branch was derived from the title AND create_new_branch is true; explicit names still produce a loud failure on collision. - request_creation pre-resolves the session title using the same logic the builder runs, so an empty title no longer flickers from the path basename to a civilization name when the real instance replaces the stub. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Note: this comment was drafted by Claude via back-and-forth with @njbrake. The reasoning and decisions are his; the prose is Claude's. Thanks for the review! I think TUI-only is the right scope for this PR. The web dialog has its own component structure, and an API change is needed to thread |
njbrake
left a comment
There was a problem hiding this comment.
Note: this review was drafted by Claude via back-and-forth with @njbrake. The reasoning and decisions are his; the prose is Claude's.
Approving. The implementation is clean: the worktree_enabled split from worktree_branch is well-modeled, the settings wiring is complete (FieldKey, SettingField, apply_to_global, apply_to_profile, clear_profile_override), and the test coverage on branch_name_from_title and resolve_worktree_branch is solid.
I pushed three follow-up commits to your branch to harden edge cases that came up during review:
- Latin diacritic fold in
branch_name_from_titleso titles likecaféproducecafeinstead of silently dropping the accented characters. - Collision dedup for title-derived branches: when the branch was derived from the title and
create_new_branchis true, suffix-2,-3, etc. against existing git branches and active aoe sessions. Explicit names still surface a loud failure on collision so the user knows to rename. - Pre-resolve the title in
request_creationso the Creating stub and the final session show the same name (previously an empty title flickered from path basename to civilization name).
Web UI parity is tracked separately in #980 so this can land without expanding scope.
Thanks for the well-tested PR and the clear screenshots.
The 35-arm hardcoded match was a maintenance hazard and incomplete. NFKD decomposition handles accented characters (é → e + combining acute, then the combining mark is filtered out as non-ASCII), so the only thing the table needs to cover is Latin ligatures and stroked letters that have no canonical decomposition (ß, æ, œ, ø, ł, đ, þ). This also picks up Vietnamese, Polish ąęś, Turkish dotted I, and other accented Latin variants that the previous table missed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Glad you liked it! Yup, happy to work on Web UI parity too 👌 |
Description
Slightly opinionated change to smoothen up session creation for worktree-driven workflows.
Problem: Adding a new session in worktrees required manually setting session title AND worktree. Too many clicks.
This PR adds a Worktree checkbox to the new-session TUI flow so worktree creation can be enabled independently of an explicit branch/name. When enabled with no Worktree configuration Name, the branch name is derived from the session title. The Worktree configuration modal now mirrors the Sandbox configuration flow and exposes Name, New Branch, and Extra Repos.
Also adds a Worktree settings toggle for enabling worktree mode by default, plus docs/tests for the title-derived behavior.
Effectively removes friction. The workflow for working in worktrees becomes Enable by default in settings -> Just add title. Done. Mirrors behaviour of Docker sandbox.
Screenshots:
PR Type
Checklist
AI Usage
AI Model/Tool used:
Codex
Any Additional AI Details you'd like to share:
Implemented via an interactive coding session with local review, tests, and manual screenshot verification.
NOTE:
When responding to reviewer questions, please respond yourself rather than copy/pasting reviewer comments into an AI and pasting back its answer. We want to discuss with you, not your AI :)
Testing
cargo clippy -- -D warningscargo fmt -- --checkcargo test --quiet