Re: [PATCH 2/2] mergetool: Don't assume paths are unmerged

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 22, 2011, 21:26 UTC
Message-ID: <7v62nx5zhk.fsf@alter.siamese.dyndns.org>
In-Reply-To: <92B6FB42-FE0D-48DC-ABD0-BA1903D842D2@JonathonMah.com>

Jonathon Mah <me@JonathonMah.com> writes:

Show 9 quoted lines

> Like commit, mergetool now treats its path arguments as restricting
> operation to the listed paths. Running "git mergetool subdir" will
> prompt to resolve all conflicted blobs under subdir.
>
> Previously mergetool would assume each path was in an unresolved state,
> and get confused when it couldn't check out their other stages.
>
> Signed-off-by: Jonathon Mah <me@JonathonMah.com>
> ---

This does two unrelated things, no?

 - Given the name of a file from the command line, current mergetool gets
   confused if the given file is not in a conflicted state (bugfix).

 - Instead of taking command line arguments as literal filenames, treat
   them as pathspec like any sane git subcommand does, allowing the users
   more flexibility (new feature).

And the title of the patch advertises only the first point. Using the command line argument as pathspec may make a lot of sense, and code reduction in git-mergetool.sh with this patch may also be a good change.

Show 15 quoted lines

> diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
> index 8c79ae8..f1f4e7a 100644
> --- a/Documentation/git-mergetool.txt
> +++ b/Documentation/git-mergetool.txt
> @@ -16,9 +16,10 @@ Use `git mergetool` to run one of several merge utilities to resolve
>  merge conflicts.  It is typically run after 'git merge'.
>  
>  If one or more <file> parameters are given, the merge tool program will
> -be run to resolve differences on each file.  If no <file> names are
> -specified, 'git mergetool' will run the merge tool program on every file
> -with merge conflicts.
> +be run to resolve differences on each file (skipping those without
> +conflicts).  Specifying a directory will include all unresolved files in
> +that path.  If no <file> names are specified, 'git mergetool' will run
> +the merge tool program on every file with merge conflicts.

The documentation may also have to reword <file> written back in the days when they were literal filenames, not pathspecs.

> +	matches=$(git ls-files -u -- "$1" | sed -e 's/^[^	]*	//' | sort -u)

Would we want to catch and signal a typo like this?

	git mergetool Mkaefile
	git mergetool Documentaiton/

when there is a conflict in Makefile and some file in Documentation/ directory, and user obviously wanted to name them but botched typing? I think you are just letting these typos silently go.

Show 6 quoted lines

> @@ -392,7 +410,7 @@ test_expect_success 'directory vs modified submodule' '
>      test "$(cat submod/file16)" = "not a submodule" &&
>      rm -rf submod.orig &&
>  
> -    git reset --hard &&
> +    git reset --hard >/dev/null 2>&1 &&

What is the justification for this change?

Please do not make "sh txxx.sh -v" less useful.