| Message ID | 20181129215850.7278-12-pclouds@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce new commands switch-branch and restore-files | expand |
On Thu, Nov 29, 2018 at 10:58:46PM +0100, Nguyễn Thái Ngọc Duy wrote: > + if (!opts->implicit_detach && > + !opts->new_branch && > + !opts->new_branch_force && > + new_branch_info->name && > + !new_branch_info->path) > + die(_("a branch is expected, got %s"), new_branch_info->name); Wouldn't it be nice to give more context here, for example the symbolic reference that the name actually points to? When expereimenting with the feature and trying to switch to a tag, it refuses with "a branch is expected, got v1.2.0". I personally would prefer something more like "a branch is expected, got v1.2.0 that resolved to refs/tags/v1.2.0", so I get "oh, yeah, that is actually a tag ...". Does this seem worthwhile to dig deeper into? A quick glance left me a bit puzzled, I admit. Greetings, Eckhard
On Mon, Mar 11, 2019 at 2:32 AM Eckhard Maaß <eckhard.s.maass@googlemail.com> wrote: > > On Thu, Nov 29, 2018 at 10:58:46PM +0100, Nguyễn Thái Ngọc Duy wrote: > > + if (!opts->implicit_detach && > > + !opts->new_branch && > > + !opts->new_branch_force && > > + new_branch_info->name && > > + !new_branch_info->path) > > + die(_("a branch is expected, got %s"), new_branch_info->name); > > Wouldn't it be nice to give more context here, for example the symbolic > reference that the name actually points to? When expereimenting with the > feature and trying to switch to a tag, it refuses with > "a branch is expected, got v1.2.0". I personally would prefer something > more like "a branch is expected, got v1.2.0 that resolved to > refs/tags/v1.2.0", so I get "oh, yeah, that is actually a tag ...". Does > this seem worthwhile to dig deeper into? A quick glance left me a bit > puzzled, I admit. Good suggestion. I'll try to report one of the following a branch is expected, got tag 'v1.2.0' a branch is expected, got remote branch 'origin/master' a branch is expected, got 'refs/foo/bar' a branch is expected, got commit 'HEAD^' It's a bit more code, but I think it definitely helps.
diff --git a/builtin/checkout.c b/builtin/checkout.c index c7ae068d2c..fbfebba2d9 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -49,6 +49,7 @@ struct checkout_opts { int merge; int force; int force_detach; + int implicit_detach; int writeout_stage; int overwrite_ignore; int ignore_skipworktree; @@ -1241,6 +1242,13 @@ static int checkout_branch(struct checkout_opts *opts, !opts->force_detach) die(_("nothing to do")); + if (!opts->implicit_detach && + !opts->new_branch && + !opts->new_branch_force && + new_branch_info->name && + !new_branch_info->path) + die(_("a branch is expected, got %s"), new_branch_info->name); + if (new_branch_info->path && !opts->force_detach && !opts->new_branch && !opts->ignore_other_worktrees) { int flag; @@ -1485,6 +1493,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) opts.dwim_new_local_branch = 1; opts.switch_branch_doing_nothing_not_ok = 0; opts.accept_pathspec = 1; + opts.implicit_detach = 1; options = parse_options_dup(checkout_options); options = add_common_options(&opts, options); @@ -1513,6 +1522,7 @@ int cmd_switch_branch(int argc, const char **argv, const char *prefix) opts.dwim_new_local_branch = 1; opts.accept_pathspec = 0; opts.switch_branch_doing_nothing_not_ok = 1; + opts.implicit_detach = 0; options = parse_options_dup(switch_options); options = add_common_options(&opts, options);
"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-branch a bit more friendly to new users, we only allow entering detached HEAD mode when --detach is given. "git switch-branch" must take a branch (unless you create a new branch, then of course switch-branch can take any commit-ish) Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- builtin/checkout.c | 10 ++++++++++ 1 file changed, 10 insertions(+)