Skip to content

Fix default worktree location fallback#371

Open
qzchenwl wants to merge 1 commit intoasheshgoplani:mainfrom
qzchenwl:worktree-default-location
Open

Fix default worktree location fallback#371
qzchenwl wants to merge 1 commit intoasheshgoplani:mainfrom
qzchenwl:worktree-default-location

Conversation

@qzchenwl
Copy link
Contributor

Summary

  • make the default worktree location fallback to sibling
  • keep empty location semantics aligned with sibling
  • add coverage for config files that omit default_location

Testing

  • go test ./internal/session -run 'TestGetWorktreeSettings|TestGetWorktreeSettings_FromConfig|TestGetWorktreeSettings_UsesSiblingWhenLocationUnsetInConfig'
  • go test ./internal/git -run 'TestGenerateWorktreePath|TestWorktreePath'

@asheshgoplani
Copy link
Owner

Thanks for the PR! However, this changes the default worktree location from 'subdirectory' to 'sibling' for all users who haven't explicitly set it. Our policy is that defaults must not change without a strong reason and migration plan.

Could you either:

  1. Keep 'subdirectory' as the default and make 'sibling' opt-in via config
  2. Or explain why 'sibling' is strictly better as the default (e.g. git issues with subdirectory approach)

Happy to merge once the default behavior is preserved.

@qzchenwl
Copy link
Contributor Author

Thanks, that’s fair. The main issue I was trying to address is that the current behavior and the published docs are inconsistent.

Right now:

  • the runtime fallback is subdirectory
  • README says sibling is the default
  • the generated config template also says sibling

So the inconsistency is real; this PR chose to resolve it by aligning the implementation with the docs. If you want to preserve the current runtime behavior instead, then I think the right fix is to update the docs/template to consistently say subdirectory is the default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants