Re: [PATCH v8 3/4] worktree add: add --orphan flag

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Jacob

On 14/01/2023 22:47, Jacob Abel wrote:
On 23/01/13 10:20AM, Phillip Wood wrote:
Hi Jacob

On 09/01/2023 17:33, Jacob Abel wrote:
[...]

It's perhaps a bit late to bring this up but I've only just realized
that it is unfortunate that --orphan takes a branch name rather than
working in conjunction with -b/-B. It means that in the common case
where the branch name is the same as the worktree the user has to repeat
it on the command line as shown above. It also means there is no way to
force an orphan branch (that's admittedly quite niche). If instead
--orphan did not take an argument we could have

	git worktree add --orphan main
	git worktree add --orphan -b topic main
	git worktree add --orphan -B topic main

Best Wishes

Phillip

[...]

I think this is a good idea and something similar was brought up previously
however I originally wanted to handle this and a common --orphan DWYM in a later
patch.

I think adding it in a later patch makes the implementation more complicated than it needs to be as you'll still have to support --orphan taking a name for backwards compatibility that means you need to handle

	git worktree add --orphan=main main
	git worktree add --orphan topic main
	git worktree add --orphan --lock main
	git worktree add --orphan -b topic main
	git worktree add --orphan -B topic main

Rather than just the last three. Now you can probably get that to work by changing --orphan to take an optional argument but I think it would be simpler to have --orphan as a flag from the start.

	git worktree add --orphan main

I am OK implementing this option and have been workshopping it prior to
responding. I think I have it worked out now as an additional patch which can be
be applied on top of the v8 patchset.

I'll reply to this message with the one-off patch to get feedback. Since this is
essentially a discrete change on top of v8, I can either keep it as a separate
patch or reroll depending on how much needs to be changed (and what would be
easier for everyone).

	git worktree add --orphan -b topic main
	git worktree add --orphan -B topic main

I am hesitant to add these as they break away from the syntax used in
`git switch` and `git checkout`.

When I wrote my original email I wrongly though that --orphan did not take an argument for "git checkout". While I think it is a mistake for checkout and switch to have --orphan take an argument they do at least always need a branch name with that option. "git worktree" add already has the branch name in the form of the worktree directory in the common case.

Also apologies for the tangent but while researching this path, I noticed that
--orphan behaves unexpectedly on both `git switch` and `git checkout` when mixed
with `-c` and `-b` respectively.

     % git switch --orphan -c foobar
     fatal: invalid reference: foobar

     % git switch -c --orphan foobar
     fatal: invalid reference: foobar
>      % git checkout -b --orphan foobar
     fatal: 'foobar' is not a commit and a branch '--orphan' cannot be created from it

     % git checkout --orphan -b foobar
     fatal: 'foobar' is not a commit and a branch '-b' cannot be created from it

The messages for checkout look better than the switch ones to me as they show the branch name which makes it clearer that we're treating what looks like an option as an argument. What in particular is unexpected here - --orphan and -b take an argument so they'll hoover up the next thing on the commandline whatever it is.

Best Wishes

Phillip

I tried this on my system install as well as from a fresh fetch of next FWIW.

[Info: fresh build from next]
git version 2.39.0.287.g8cbeef4abd
cpu: x86_64
built from commit: 8cbeef4abda4907dd68ea144d9dcb85f0b49c3e6
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh

[Info: system install]
git version 2.38.2
cpu: x86_64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh

If this bug is something that needs to be addressed, I can dig a bit deeper and
put together a patch for it in the next few days.

VR,
Abel




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux