Thread (13 messages) flat view 13 messages, 5 authors, 2021-02-23

Re: New orphan worktree?

From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2021-02-23 11:06:55

On Tue, Feb 23 2021, Eric Sunshine wrote:
On Mon, Feb 22, 2021 at 7:17 PM Ævar Arnfjörð Bjarmason
[off-list ref] wrote:
quoted
On Tue, Feb 23 2021, Eric Sunshine wrote:
quoted
I'm not sure I follow. In git-switch, --orphan does not imply -c even
though --orphan also creates a new branch (thus seems to work similar
to -c); it is nevertheless mutually-exclusive with -c and -C. The same
goes for --orphan in git-branch.
I think we're on the same page with regards to what I meant. I.e. I
don't see how it makes sense to conflate the type of branch we want
(orphan or not orphan) with whether we want to clobber that branch or
not (switch -c or -C, or worktree -b or -B)
I see where you're coming from in viewing --orphan as a modifier of
branch creation rather than as a branch-creation option itself.
However, as far as UI is concerned, that ship sailed a long time ago,
I suppose.
Not really, I think we can have a new-style of it and just say:

    It is also possible to provide `--orphan <branch-name>`, but
    supplying it as an option to `-[cC]` as `-[cC] <branch-name>
    --orphan` is preferred these days.

Whether we should is another matter, see below...
quoted
quoted
As far as combining --orphan and -C (or -c), I'm not sure how we would
arrange that using the existing parse_options() mechanism. It seems
too magical and has potential for weird corner cases.
Isn't it just having --orphan be an OPTION_STRING with
PARSE_OPT_LASTARG_DEFAULT. I.e. to support:

    git switch -b branch --orphan
    git switch -B branch --orphan
    git switch --orphan branch

And:

    git worktree add -b branch --orphan
    git worktree add -B branch --orphan

I didn't test it, just skimmed the code.
I haven't dived into this stuff in a long time, but I'm having trouble
convincing myself that it would work out as intended. If I'm reading
PARSE_OPT_LASTARG_DEFAULT correctly, `git switch -b <branch> --orphan`
would not be the same as `git switch --orphan -b <branch>`
Yeah, I think so. But I think for an option like that it would be more
obvious. I.e. we could say:

    If "-b" or "-B" is provided a subsequent "--orphan" is a boolean.

We don't support the combination of the two now, so we could just
mandate that the order matters.

Anyway...
, and I don't think it would work at all for git-worktree-add which
has additional <path> and <commitish> arguments (i.e. `git worktree
add -b <branch> --orphan <path> [<commitish>]`).
...we can parse these options, whether it's easy or trivial with
parse-options.c is something I'd like to leave aside for now.

Right now I'm not intending to re-roll this patch, but maybe someone
else (or even me) will get to it sometime. I think it's more useful
if/when that happens to get people's take on whether this makes sense as
UI, not whether it's trivial with the current parse_options() API.

I think it's fairly easy to tease this behavior out of
parse_options(). Worse case we can do a pre-loop over argv and see if
both "--orphan" and "-b"/"-B" occur. if so parse it with "--orphan" as a
BOOL, otherwise STRING.
Anyhow, as I responded elsewhere to Junio, my present leaning is
toward -b, -B, --orphan all being mutually-exclusive branch-creation
options, each taking a <branch> argument -- just like they are in
git-checkout and git-switch (-c/-C, in this case) -- and allowing
--force to overwrite an existing branch (in which case, -B can be
viewed as shorthand for `--force -b`).
See https://lore.kernel.org/git/7vpqzlrmo4.fsf@alter.siamese.dyndns.org/ (local)
for past Junio arguing with his future self :)

I.e. the reason we had -B in the first place is because --force means
something else. We'd need "--force-ref-deletion
--force-work-tree-clobbering" or whatever, or "--force --force".

I like this lower/upper case convention. It started with "branch -D" in
ba65af9c1f6 (git-branch -d <branch>: delete unused branch., 2005-09-14),
but in checkout the --orphan option pre-dates -B. See 9db5ebf4022 (git
checkout: create unparented branch by --orphan, 2010-03-21) and
02ac98374ee (builtin/checkout: learn -B, 2010-06-24).

I don't think we're going to change how "branch -D" and "switch -C" work
at this point, so making things consistent with it makes sense.
quoted
quoted
Since git-worktree doesn't yet support --orphan, we certainly have
more leeway and could go with your proposal of having --orphan be
boolean and always requiring it to be used in conjunction with -b/-B.
However, I'm quite hesitant to take that approach since it breaks with
existing precedent in git-branch and git-switch, in which case
--orphan takes its own argument (<branch>) and is mutually-exclusive
with -b/-B/-c/-C.
In git-branch? Isn't it only git [checkout|switch] that takes --orphan?
Um, yes, I meant git-checkout everywhere I wrote git-branch. Sorry for
the confusion.
*nod*
quoted
I think not having a -B or -C equivalent at all would be preferrable to
having a --force special-case just to work around the lack of it for
--orphan.
I'm having trouble wrapping my brain around this statement.
I mean I'd rather not have an --orphan mode that works like -B (as
opposed to -b) at all instead of having one that's "--orphan
--force-ref-deletion" or whatever.

It's an obscure enough thing that I don't think anyone *really* cares. I
just wanted to find out if it not being a boolean was intentional, or a
historical accident we would consider fixing if there was further work
on it.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help