Re: [RFC/PATCH] checkout: allow dwim for branch creation for git checkout $branch --
Junio C Hamano gits...@pobox.com writes: Did anything further happen to this discussion? Is v3 the version with agreement among the list members I just should pick up? v3 ( http://thread.gmane.org/gmane.comp.version-control.git/235409/focus=235408 ) is the last version I sent, and I got no feedback on it, so I guess it's ready for you to pick. Thanks, -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] checkout: allow dwim for branch creation for git checkout $branch --
Matthieu Moy matthieu@grenoble-inp.fr writes: v3 ( http://thread.gmane.org/gmane.comp.version-control.git/235409/focus=235408 ) is the last version I sent, and I got no feedback on it, so I guess it's ready for you to pick. Thanks; done with s/pick/nitpick/ ;-). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] checkout: allow dwim for branch creation for git checkout $branch --
Matthieu Moy matthieu@grenoble-inp.fr writes: Duy Nguyen pclo...@gmail.com writes: has_dash_dash is calculated as (argc 1) !strcmp(argv[1], --), so when argc == 1, the has_dash_dash must be zero, the !has_dash_dash is redundant. Yes, but I'd rather not have to read the detailed definition of has_dash_dash to understand the code. With my version, the name of the variable is actually sufficient to understand. But it makes me wonder what we do with git checkout abc def -- xyz. Ouch. Both old and new say ... I'll resend, together with tweaks to the first patch. Did anything further happen to this discussion? Is v3 the version with agreement among the list members I just should pick up? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH] checkout: allow dwim for branch creation for git checkout $branch --
The -- notation disambiguates files and branches, but as a side-effect
of the previous implementation, also disabled the branch auto-creation
when $branch does not exist.
A possible scenario is then:
git checkout $branch
= fails if $branch is both a ref and a file, and suggests --
git checkout $branch --
= refuses to create the $branch
This patch allows the second form to create $branch, and since the -- is
provided, it does not look for file named $branch.
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
builtin/checkout.c | 32
t/t2024-checkout-dwim.sh | 22 ++
2 files changed, 46 insertions(+), 8 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 0f57397..f1f5915 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -863,6 +863,14 @@ static const char *unique_tracking_name(const char *name,
unsigned char *sha1)
return NULL;
}
+static int error_invalid_ref(const char *arg, int has_dash_dash, int argcount)
+{
+ if (has_dash_dash)
+ die(_(invalid reference: %s), arg);
+ else
+ return argcount;
+}
+
static int parse_branchname_arg(int argc, const char **argv,
int dwim_new_local_branch_ok,
struct branch_info *new,
@@ -916,20 +924,28 @@ static int parse_branchname_arg(int argc, const char
**argv,
if (!strcmp(arg, -))
arg = @{-1};
- if (get_sha1_mb(arg, rev)) {
- if (has_dash_dash) /* case (1) */
- die(_(invalid reference: %s), arg);
- if (dwim_new_local_branch_ok
- !check_filename(NULL, arg)
- argc == 1) {
+ if (get_sha1_mb(arg, rev)) { /* case (1)? */
+ int try_dwim = dwim_new_local_branch_ok;
+
+ if (check_filename(NULL, arg) !has_dash_dash)
+ try_dwim = 0;
+ /*
+* Accept git checkout foo and git checkout foo --
+* as candidates for dwim.
+*/
+ if (!(argc == 1 !has_dash_dash)
+ !(argc == 2 has_dash_dash))
+ try_dwim = 0;
+
+ if (try_dwim) {
const char *remote = unique_tracking_name(arg, rev);
if (!remote)
- return argcount;
+ return error_invalid_ref(arg, has_dash_dash,
argcount);
*new_branch = arg;
arg = remote;
/* DWIMmed to create local branch */
} else {
- return argcount;
+ return error_invalid_ref(arg, has_dash_dash, argcount);
}
}
diff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh
index 094b92e..d9afdb2 100755
--- a/t/t2024-checkout-dwim.sh
+++ b/t/t2024-checkout-dwim.sh
@@ -164,4 +164,26 @@ test_expect_success 'checkout of branch from a single
remote succeeds #4' '
test_branch_upstream eggs repo_d eggs
'
+test_expect_success 'checkout of branch with a file having the same name
fails' '
+ git checkout -B master
+ test_might_fail git branch -D spam
+
+ spam
+ test_must_fail git checkout spam
+ test_must_fail git checkout spam
+ test_must_fail git rev-parse --verify refs/heads/spam
+ test_branch master
+'
+
+test_expect_success 'checkout branch -- succeeds, even if a file with the
same name exists' '
+ git checkout -B master
+ test_might_fail git branch -D spam
+
+ spam
+ git checkout spam --
+ test_branch spam
+ test_cmp_rev refs/remotes/extra_dir/repo_c/extra_dir/spam HEAD
+ test_branch_upstream spam repo_c spam
+'
+
test_done
--
1.8.4.475.g703937e.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] checkout: allow dwim for branch creation for git checkout $branch --
On Wed, Sep 25, 2013 at 7:49 PM, Matthieu Moy matthieu@imag.fr wrote:
static int parse_branchname_arg(int argc, const char **argv,
int dwim_new_local_branch_ok,
struct branch_info *new,
You may want to update the big comment block about all cases at the
beginning of this function.
@@ -916,20 +924,28 @@ static int parse_branchname_arg(int argc, const char
**argv,
if (!strcmp(arg, -))
arg = @{-1};
- if (get_sha1_mb(arg, rev)) {
- if (has_dash_dash) /* case (1) */
- die(_(invalid reference: %s), arg);
- if (dwim_new_local_branch_ok
- !check_filename(NULL, arg)
- argc == 1) {
+ if (get_sha1_mb(arg, rev)) { /* case (1)? */
+ int try_dwim = dwim_new_local_branch_ok;
+
+ if (check_filename(NULL, arg) !has_dash_dash)
+ try_dwim = 0;
+ /*
+* Accept git checkout foo and git checkout foo --
+* as candidates for dwim.
+*/
+ if (!(argc == 1 !has_dash_dash)
has_dash_dash is calculated as (argc 1) !strcmp(argv[1], --),
so when argc == 1, the has_dash_dash must be zero, the
!has_dash_dash is redundant. But it makes me wonder what we do with
git checkout abc def -- xyz. I think both old and new code do not
see -- as a separator like it should (and should also error about
multiple refs given). Maybe something worth fixing too while you're
updating this code.
+ !(argc == 2 has_dash_dash))
+ try_dwim = 0;
+
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] checkout: allow dwim for branch creation for git checkout $branch --
On Wed, Sep 25, 2013 at 8:52 PM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
@@ -925,7 +926,15 @@ static int parse_branchname_arg(int argc, const char
**argv,
return 1;
arg = argv[0];
- has_dash_dash = (argc 1) !strcmp(argv[1], --);
+ has_dash_dash = 0;
+ for (i = 0; i argc; i++) {
+ if (!strcmp(argv[i], --)) {
+ has_dash_dash = i;
+ break;
+ }
+ }
+ if (has_dash_dash = 2)
+ die(only one reference expected, %d given., has_dash_dash);
The translator in me says this string should be marked for translation
like others in git-checkout...
if (!strcmp(arg, -))
arg = @{-1};
diff --git a/t/t2010-checkout-ambiguous.sh b/t/t2010-checkout-ambiguous.sh
index 7cc0a35..2836a3e 100755
--- a/t/t2010-checkout-ambiguous.sh
+++ b/t/t2010-checkout-ambiguous.sh
@@ -47,4 +47,10 @@ test_expect_success 'disambiguate checking out from a
tree-ish' '
git diff --exit-code --quiet
'
+test_expect_success 'accurate error message with more than one ref' '
+ test_must_fail git checkout HEAD master -- 2actual
+ echo fatal: only one reference expected, 2 given. expect
+ test_cmp expect actual
+'
+
test_done
which makes C_LOCALE_OUTPUT a prerequisite for this test because it
needs the untranslated version of the string.
I'll resend, together with tweaks to the first patch.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
