Hi,
On Wed, 23 Jul 2008, Pierre Habouzit wrote:
> Note that it also fix a bug, git checkout -- <path> would be badly
s/fix/fixes/
> understood as git checkout <branch> if <path> is ambiguous with a branch.
>
> Testcases included.
>
> Signed-off-by: Pierre Habouzit <madcoder@xxxxxxxxxx>
Instead of the "Testcases included", I would have expected something like
When calling "git checkout <name>" and <name> is both a path and a
branch, Git would silently assume that you meant the branch.
Worse, "git checkout -- <name>" would behave the same.
probably even as first paragraph.
> This is a resend with proper patches that made me realize that my
s/patches/tests/
> previous patch had silly mistakes and wasn't testing thigns properly.
>
> builtin-checkout.c | 23 +++++++++++++++++------
> t/t2010-checkout-ambiguous.sh | 35 +++++++++++++++++++++++++++++++++++
> 2 files changed, 52 insertions(+), 6 deletions(-)
> create mode 100755 t/t2010-checkout-ambiguous.sh
>
> diff --git a/builtin-checkout.c b/builtin-checkout.c
> index fbd5105..97321e6 100644
> --- a/builtin-checkout.c
> +++ b/builtin-checkout.c
> @@ -438,12 +438,17 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
>
> opts.track = git_branch_track;
>
> - argc = parse_options(argc, argv, options, checkout_usage, 0);
> - if (argc) {
> + argc = parse_options(argc, argv, options, checkout_usage,
> + PARSE_OPT_KEEP_DASHDASH);
> +
> + if (argc && strcmp(argv[0], "--")) {
> + int may_be_ambiguous = argc == 1 || strcmp(argv[1], "--");
> +
> arg = argv[0];
> - if (get_sha1(arg, rev))
> - ;
> - else if ((new.commit = lookup_commit_reference_gently(rev, 1))) {
> + if (get_sha1(arg, rev)) {
> + if (may_be_ambiguous)
> + verify_filename(NULL, arg);
Still you allow "git checkout <path> --" to succeed, right? It should
not.
IMHO you need "must_be_branch" and "must_not_be_path" instead of
"may_be_ambiguous".
I.e. something like
int must_be_branch = argc > 1 && !strcmp(argv[1], "--");
int must_not_be_path = argc > 1 && strcmp(argv[1], "--");
arg = argv[0];
if (get_sha1(arg, rev)) {
if (must_be_branch)
die ("Not a branch: '%s'", arg);
}
else {
if (must_not_be_path)
verify_non_filename(NULL, arg);
if ((new.commit = lookup_commit_reference_gently(rev, 1))) {
[...]
And maybe you want to add a test for "git checkout <path> --" to fail,
too.
Ciao,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
