diff mbox series

checkout: add config variable checkout.autoDetach

Message ID 20231111224253.1923-1-andy.koppe@gmail.com (mailing list archive)
State New, archived
Headers show
Series checkout: add config variable checkout.autoDetach | expand

Commit Message

Andy Koppe Nov. 11, 2023, 10:42 p.m. UTC
The git-checkout command without pathspecs automatically detaches HEAD
when switching to something other than a branch, whereas git-switch
requires the --detach option to do so.

Add configuration variable checkout.autoDetach to choose the behavior
for both: true for automatic detaching, false for requiring --detach.

Amend their documentation and tests accordingly.

Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
---
I like to use git-switch and git-restore instead of git-checkout, and
also recommend them when new users ask how to switch branches or undo
local changes. But I do miss git-checkout's auto-detaching, as I like
to avoid creating local versions of remote branches when I don't intend
to make changes to them.

Hence this patch, which turned out to be very simple in terms of the
actual source change, as the checkout_opts.implicit_detach field
already controls the necessary functionality. That was added in commit
7968bef06b by Nguyễn Thái Ngọc Duy, with the following explanation:

> "git checkout <commit>" will checkout the commit in question and
> detach HEAD from the current branch. It is naturally a right thing to
> do once you get git references. But detached HEAD is a scary concept
> to new users because we show a lot of warnings and stuff, and it could
> be hard to get out of (until you know better).
>
> To keep switch a bit more friendly to new users, we only allow
> entering detached HEAD mode when --detach is given.

I think that makes plenty of sense, and I don't think it conflicts with
the config setting proposed here, as new users are unlikely to mess with
such settings. In fact, the point where "you get git references" would
probably be a good time to enable checkout.autoDetach.

Conversely, admins might want to set checkout.autoDetach to false in the
system-wide config to prevent accidental decapitation by git-checkout
and the resulting support requests.

 Documentation/config/checkout.txt |  8 ++++++++
 Documentation/git-checkout.txt    |  3 +++
 Documentation/git-switch.txt      |  3 +++
 builtin/checkout.c                |  4 ++++
 t/t2020-checkout-detach.sh        | 14 ++++++++++++++
 t/t2060-switch.sh                 |  7 +++++++
 6 files changed, 39 insertions(+)

Comments

Junio C Hamano Nov. 12, 2023, 6:04 a.m. UTC | #1
Andy Koppe <andy.koppe@gmail.com> writes:

> The git-checkout command without pathspecs automatically detaches HEAD
> when switching to something other than a branch, whereas git-switch
> requires the --detach option to do so.
>
> Add configuration variable checkout.autoDetach to choose the behavior
> for both: true for automatic detaching, false for requiring --detach.
>
> Amend their documentation and tests accordingly.
>
> Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
> ---

"switch" was meant to be an experimental command to sort out this
kind of UI ideas, and I think the fact that it requires a more
explicit "--detach", where experienced users might just say "git
checkout that-branch^0", has established itself as a more friendly
and good thing to help new users.  I do not know how others react to
this kind of proliferation of configuration variables, but I do not
mind this particular variable existing.
Andy Koppe Nov. 12, 2023, 9:15 a.m. UTC | #2
On 12/11/2023 06:04, Junio C Hamano wrote:
> Andy Koppe <andy.koppe@gmail.com> writes:
> 
>> The git-checkout command without pathspecs automatically detaches HEAD
>> when switching to something other than a branch, whereas git-switch
>> requires the --detach option to do so.
>>
>> Add configuration variable checkout.autoDetach to choose the behavior
>> for both: true for automatic detaching, false for requiring --detach.
>>
>> Amend their documentation and tests accordingly.
>>
>> Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
>> ---
> 
> "switch" was meant to be an experimental command to sort out this
> kind of UI ideas, and I think the fact that it requires a more
> explicit "--detach", where experienced users might just say "git
> checkout that-branch^0", has established itself as a more friendly
> and good thing to help new users. 

I agree, but as an experienced user, I nevertheless prefer switch and 
restore over checkout, because those are rather different tasks, and 
with checkout you're only ever a small thinko and errant dot away from 
losing your local changes. If switch and restore had existed first, I 
don't think anyone would be asking for mashing them together.

Incidentally, as reset is similarly overloaded, and restore can also 
replace the forms of reset that take pathspec arguments, was there a 
similar plan to factor the head-moving forms of reset out into a 
separate command? (I realise there'd be little appetite for that after 
the switch/restore experiment.)

> I do not know how others react to
> this kind of proliferation of configuration variables, but I do not
> mind this particular variable existing.

Thanks. There's also the checkout.guess variable as a closely related 
precedent.

Regards,
Andy
Phillip Wood Nov. 13, 2023, 3:07 p.m. UTC | #3
On 12/11/2023 06:04, Junio C Hamano wrote:
> Andy Koppe <andy.koppe@gmail.com> writes:
> 
>> The git-checkout command without pathspecs automatically detaches HEAD
>> when switching to something other than a branch, whereas git-switch
>> requires the --detach option to do so.
>>
>> Add configuration variable checkout.autoDetach to choose the behavior
>> for both: true for automatic detaching, false for requiring --detach.
>>
>> Amend their documentation and tests accordingly.
>>
>> Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
>> ---
> 
> "switch" was meant to be an experimental command to sort out this
> kind of UI ideas, and I think the fact that it requires a more
> explicit "--detach", where experienced users might just say "git
> checkout that-branch^0", has established itself as a more friendly
> and good thing to help new users.  I do not know how others react to
> this kind of proliferation of configuration variables, but I do not
> mind this particular variable existing.

I'm a bit wary of having a config variable that could break scripts 
relying on the current behavior of "git checkout". As far as "git 
switch" goes I don't particularly mind this config variable though I'm 
not sure it is that hard to type "--detach" (especially with tab 
completion) and I do worry that we're making the UI more complex each 
time we add something like this.

Best Wishes

Phillip
Junio C Hamano Nov. 14, 2023, 12:48 a.m. UTC | #4
Phillip Wood <phillip.wood123@gmail.com> writes:

>> and good thing to help new users.  I do not know how others react to
>> this kind of proliferation of configuration variables, but I do not
>> mind this particular variable existing.
>
> I'm a bit wary of having a config variable that could break
> scripts relying on the current behavior of "git checkout".  As far
> as "git switch" goes I don't particularly mind this config
> variable though I'm not sure it is that hard to type "--detach"
> (especially with tab completion) and

I do not have much sympathy myself to scripts that are not being
defensive enough to write "--detach" explicitly, but I do understand
and share your concern as the project maintainer.

> ... I do worry that we're making the UI more complex each
> time we add something like this.

Thanks for saying this---this is exactly the kind of reaction I as
expecting to see.

>
> Best Wishes
>
> Phillip
diff mbox series

Patch

diff --git a/Documentation/config/checkout.txt b/Documentation/config/checkout.txt
index a323022993..6827ee74d5 100644
--- a/Documentation/config/checkout.txt
+++ b/Documentation/config/checkout.txt
@@ -17,6 +17,14 @@  and by linkgit:git-worktree[1] when `git worktree add` refers to a
 remote branch. This setting might be used for other checkout-like
 commands or functionality in the future.
 
+checkout.autoDetach::
+	If set to true, `git checkout` and `git switch` automatically detach
+	HEAD when switching to something other than a branch. If set to false,
+	they require the `--detach` option to detach HEAD.
++
+If this setting is not specified, `git checkout` defaults to automatic
+detaching, whereas `git switch` defaults to requiring `--detach`.
+
 checkout.guess::
 	Provides the default value for the `--guess` or `--no-guess`
 	option in `git checkout` and `git switch`. See
diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index 240c54639e..23f90c15ac 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -79,6 +79,9 @@  When the `<commit>` argument is a branch name, the `--detach` option can
 be used to detach `HEAD` at the tip of the branch (`git checkout
 <branch>` would check out that branch without detaching `HEAD`).
 +
+If the `checkout.autoDetach` config variable is set to false, the `--detach`
+option is required even if the `<commit>` argument is not a branch name.
++
 Omitting `<branch>` detaches `HEAD` at the tip of the current branch.
 
 'git checkout' [-f|--ours|--theirs|-m|--conflict=<style>] [<tree-ish>] [--] <pathspec>...::
diff --git a/Documentation/git-switch.txt b/Documentation/git-switch.txt
index c60fc9c138..f6b925c43b 100644
--- a/Documentation/git-switch.txt
+++ b/Documentation/git-switch.txt
@@ -83,6 +83,9 @@  $ git switch <new-branch>
 	Switch to a commit for inspection and discardable
 	experiments. See the "DETACHED HEAD" section in
 	linkgit:git-checkout[1] for details.
++
+If the `checkout.autoDetach` configuration variable is set to true,
+`--detach` can be omitted when the target is not a branch.
 
 --guess::
 --no-guess::
diff --git a/builtin/checkout.c b/builtin/checkout.c
index f02434bc15..d042638bb0 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1205,6 +1205,10 @@  static int git_checkout_config(const char *var, const char *value,
 		handle_ignore_submodules_arg(&opts->diff_options, value);
 		return 0;
 	}
+	if (!strcmp(var, "checkout.autodetach")) {
+		opts->implicit_detach = git_config_bool(var, value);
+		return 0;
+	}
 	if (!strcmp(var, "checkout.guess")) {
 		opts->dwim_new_local_branch = git_config_bool(var, value);
 		return 0;
diff --git a/t/t2020-checkout-detach.sh b/t/t2020-checkout-detach.sh
index 8202ef8c74..b842b6cc89 100755
--- a/t/t2020-checkout-detach.sh
+++ b/t/t2020-checkout-detach.sh
@@ -69,12 +69,26 @@  test_expect_success 'checkout ref^0 detaches' '
 	check_detached
 '
 
+test_expect_success 'checkout of tag with autoDetach=false fails' '
+	reset &&
+	test_config checkout.autoDetach false &&
+	test_must_fail git checkout tag &&
+	check_not_detached
+'
+
 test_expect_success 'checkout --detach detaches' '
 	reset &&
 	git checkout --detach branch &&
 	check_detached
 '
 
+test_expect_success 'checkout --detach with autoDetach=false detaches' '
+	reset &&
+	test_config checkout.autoDetach false &&
+	git checkout --detach branch &&
+	check_detached
+'
+
 test_expect_success 'checkout --detach without branch name' '
 	reset &&
 	git checkout --detach &&
diff --git a/t/t2060-switch.sh b/t/t2060-switch.sh
index e247a4735b..69ff197d11 100755
--- a/t/t2060-switch.sh
+++ b/t/t2060-switch.sh
@@ -50,6 +50,13 @@  test_expect_success 'switch and detach current branch' '
 	test_must_fail git symbolic-ref HEAD
 '
 
+test_expect_success 'switch with checkout.autoDetach=true' '
+	test_when_finished git switch main &&
+	test_config checkout.autoDetach true &&
+	git switch main^{commit} &&
+	test_must_fail git symbolic-ref HEAD
+'
+
 test_expect_success 'switch and create branch' '
 	test_when_finished git switch main &&
 	git switch -c temp main^ &&