[PATCH] checkout: disambiguate dwim tracking branches and local files
- From
Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
- Date
- Nov 10, 2018, 12:07 UTC
- Message-ID
- <20181110120707.25846-1-pclouds@gmail.com>
When checkout dwim is added in [1], it is restricted to only dwim when certain conditions are met and fall back to default checkout behavior otherwise. It turns out falling back could be confusing. One of the conditions to turn
git checkout frotz
to
git checkout -b frotz origin/frotz
is that frotz must not exist as a file. But when the user comes to expect "git checkout frotz" to create the branch "frotz" and there happens to be a file named "frotz", git's silently reverting "frotz" file content is not helping. This is reported in Git mailing list [2] and even used as an example of "Git is bad" elsewhere [3].
We normally try to do the right thing, but when there are multiple "right things" to do, it's best to leave it to the user to decide. Check this case, ask the user to use "--" to disambiguate.
[1] 70c9ac2f19 (DWIM "git checkout frotz" to "git checkout -b frotz
origin/frotz" - 2009-10-18)
[2] https://public-inbox.org/git/CACsJy8B2TVr1g+k+eSQ=pBEO3WN4_LtgLo9gpur8X7Z9GOFL_A@mail.gmail.com/
[3] https://news.ycombinator.com/item?id=18230655Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- builtin/checkout.c | 15 ++++++++++++--- t/t2024-checkout-dwim.sh | 31 +++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 3 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c index acdafc6e4c..17d48166d1 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -1079,9 +1079,15 @@ static int parse_branchname_arg(int argc, const char **argv, */ int recover_with_dwim = dwim_new_local_branch_ok; - if (!has_dash_dash && - (check_filename(opts->prefix, arg) || !no_wildcard(arg))) - recover_with_dwim = 0; + /* + * If both refs/remotes/origin/master and the file + * 'master'. Don't blindly go for 'master' file + * because it's ambiguous. Leave it for the user to + * decide. + */ + int disambi_local_file = !has_dash_dash && + (check_filename(opts->prefix, arg) || !no_wildcard(arg)); + /* * Accept "git checkout foo" and "git checkout foo --" * as candidates for dwim. @@ -1094,6 +1100,9 @@ static int parse_branchname_arg(int argc, const char **argv, const char *remote = unique_tracking_name(arg, rev, dwim_remotes_matched); if (remote) { + if (disambi_local_file) + die(_("'%s' could be both a local file and a tracking branch.\n" + "Please use -- to disambiguate"), arg); *new_branch = arg; arg = remote; /* DWIMmed to create local branch, case (3).(b) */ diff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh index 69b6774d10..fa0718c730 100755 --- a/t/t2024-checkout-dwim.sh +++ b/t/t2024-checkout-dwim.sh @@ -278,4 +278,35 @@ test_expect_success 'loosely defined local base branch is reported correctly' ' test_cmp expect actual ' +test_expect_success 'reject when arg could be part of dwim branch' ' + git remote add foo file://non-existent-place && + git update-ref refs/remotes/foo/dwim-arg HEAD && + echo foo >dwim-arg && + git add dwim-arg && + echo bar >dwim-arg && + test_must_fail git checkout dwim-arg && + test_must_fail git rev-parse refs/heads/dwim-arg -- && + grep bar dwim-arg +' + +test_expect_success 'disambiguate dwim branch and checkout path (1)' ' + git update-ref refs/remotes/foo/dwim-arg1 HEAD && + echo foo >dwim-arg1 && + git add dwim-arg1 && + echo bar >dwim-arg1 && + git checkout -- dwim-arg1 && + test_must_fail git rev-parse refs/heads/dwim-arg1 -- && + grep foo dwim-arg1 +' + +test_expect_success 'disambiguate dwim branch and checkout path (2)' ' + git update-ref refs/remotes/foo/dwim-arg2 HEAD && + echo foo >dwim-arg2 && + git add dwim-arg2 && + echo bar >dwim-arg2 && + git checkout dwim-arg2 -- && + git rev-parse refs/heads/dwim-arg2 -- && + grep bar dwim-arg2 +' + test_done
-- 2.19.1.1231.g84aef82467