Re: [PATCH] checkout: warn users if action is taken in ambiguous situation

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 31, 2012, 18:27 UTC
Message-ID: <7vd326or1d.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1346423934-7003-1-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> "git checkout foo" (no more arguments) always checks out existing
> branch "foo" even if "foo" exists on working directory. To avoid
> confusion to users who do not know this exception, say something along
> the action.

I do not think the extra noise "Checked out the new branch." your patch adds is necessary:

    $ git checkout master
    Already on 'master'
    $ >next
    $ git checkout next
    Switched to branch 'next'

If the user wanted to grab the contents from the index for path 'next' and deposit it to the file 'next' in the working tree, we are already telling the user that that is not what happened.

Even though I agree with you that it is a good goal to help users who want to disambiguate between branch and path differently from what the annoyance avoidance does, I do not think butchering the annoyance avoidance in parse_branchname_arg() is the right way to do so.

I wouldn't mind if the above interaction went like this:

    $ git checkout next
    Switched to branch 'next'
    hint: if you meant to check out the contents of 'next' out of
    hint: the index to the file 'next' in your working tree, say
    hint: "git checkout -- next" instead.  To squelch this advice,
    hint: "git config set advice.checkoutWarnFileAndBranchName false".

This should trigger only when the advice is not set (or set to true), and only when you have 'next' in the index. Presense or absense of 'next' in the working tree should not make a difference.

And doing it that way covers anoter valid case in which the user would want the exact same help from Git that your patch would not cover:

    $ git branch Makefile
    $ git checkout master
    Already on 'master'
    $ rm Makefile
    $ git checkout Makefile
    D	Makefile
    Switched to branch 'Makefile'

Another thing to look out for is this case:

    $ git branch Makefile master
    $ git checkout next
    Already on 'next'
    $ rm Makefile
    $ echo >>archive.c ;# different between master and next
    $ git checkout Makefile
    error: Your local changes to the following files would be overwritten by checkout:
        archive.c
    Please, commit your changes or stash them before you can switch branches.
    Aborting

We would want the exact same hint (with the obvious s/next/Makefile/) after the above output.

Just for a reference, with your patch, I think the output would like this (I didn't apply nor run it):

    $ git checkout Makefile
    warning: ambiguous argument 'Makefile': both revision and filename
    Use '--' to separate paths from revisions, like this:
    'git <command> [<reviosion>....] -- [<file>...]'
    warning: checked out the new branch.
    error: Your local changes...
        archive.c
    Please, commit your ...
    Aborting

which would be far worse and confusing.