On 22/11/04 01:03AM, Eric Sunshine wrote: > On Thu, Nov 3, 2022 at 9:07 PM Jacob Abel <jacobabel@xxxxxxxxxx> wrote: > > ... > > Are we sure we want to be modeling this after `git checkout --orphan`? > If I understand correctly, that option has long been considered (by > some) too clunky, which is why `git switch --orphan` was simplified to > accept only a branch name but no commit-ish, and to start the orphan > branch with an empty directory. My own feeling is that modeling it > after `git switch --orphan` is probably the way to go... I would argue that the `git checkout --orphan` command format is preferable to `git switch --orphan` when creating new worktrees. Reason being that in many cases (except when working in a new repo), if you are trying to create a worktree from an orphan you will be doing it with a different commit-ish currently checked out in your worktree than the one you want to use for the orphan (or you aren't in any worktree). Requiring the commit-ish to be inferred would limit the user to checking out an orphan from an existing worktree (in which case they could just create a new worktree normally and use `git switch --orphan` to move that to an orphan branch). > > ... > > The short help message for `git switch --orphan` and `git checkout > --orphan` say simply "new unparented branch", so this message should > probably follow suit (or consistency and to ease the job of > translators). Noted. > > ... > > Good to have these additional interlocks. I think, however, for the > sake of translators, we should use the same terminology as the > existing message above (i.e. "options ... cannot be used together"). Noted. > > > + /* > > + * From here on, new_branch will contain the branch to be checked out, > > + * and new_branch_force and opts.orphan_branch will tell us which one of > > + * -b/-B/--orphan is being used. > > + */ > > This can probably be worded a bit differently to make it clear that > from this point onward, those other variables are interpreted as if > they are booleans. Moreover, we can make this even clearer by > following the example of -B in which (by necessity due to > parse-options) the local variable in add() is a `const char *`, but > its counterpart in `struct add_opts` is a boolean (int). The one thing to note with `opts.orphan_branch` is that it is used as both a string and a boolean later in `add_worktree()`. Since orphan branches don't have any commits tied to them, we have to check out the original commit-ish in `add_worktree()` and then convert it to an orphan of name `opts.orphan_branch` instead of creating the branch prior to entering `add_worktree()` (as is done for `-B` and `-b`). I do agree that the comment should probably be re-worded. I'll update it to be clearer in v2.
