login
Search
Reroll of previously sent mergetool enhancements. Now using -y as the short option for --no-prompt, but otherwise the same as before. --
git-mergetool.sh mostly uses 8 space tabs and 4 spaces per indent. This change corrects this in a part of the file affect by a later commit in this patch series. diff -w considers this change is to be a null change. Signed-off-by: Charles Bailey <charles@hashpling.org> --- git-mergetool.sh | 38 +++++++++++++++++++------------------- 1 files changed, 19 insertions(+), 19 deletions(-) diff --git a/git-mergetool.sh b/git-mergetool.sh index 94187c3..e2da5fc 100755 --- a/git-mergetool.sh +++ b/git-mergetool.sh @@ -401,25 +401,25 @@ fi if test $# -eq 0 ; then - files=`git ls-files -u | sed -e 's/^[^ ]* //' | sort -u` - if test -z "$files" ; then - echo "No files need merging" - exit 0 - fi - echo Merging the files: "$files" - git ls-files -u | - sed -e 's/^[^ ]* //' | - sort -u | - while IFS= read i - do - printf "\n" - merge_file "$i" < /dev/tty > /dev/tty - done + files=`git ls-files -u | sed -e 's/^[^ ]* //' | sort -u` + if test -z "$files" ; then + echo "No files need merging" + exit 0 + fi + echo Merging the files: "$files" + git ls-files -u | + sed -e 's/^[^ ]* //' | + sort -u | + while IFS= read i + do + printf "\n" + merge_file "$i" < /dev/tty > /dev/tty + done else - while test $# -gt 0; do - printf "\n" - merge_file "$1" - shift - done + while test $# -gt 0; do + printf "\n" + merge_file "$1" + shift + done fi exit 0 -- 1.6.0.2.534.g5ab59 --
This option lets git mergetool invoke the conflict resolution program without waiting for a user prompt each time. Also added a mergetool.prompt (default true) configuration variable controlling the same behaviour Signed-off-by: Charles Bailey <charles@hashpling.org> --- Documentation/config.txt | 3 +++ Documentation/git-mergetool.txt | 11 ++++++++++- git-mergetool.sh | 16 +++++++++++++--- 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 965ed74..c5b211a 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -976,6 +976,9 @@ mergetool.keepBackup:: is set to `false` then this file is not preserved. Defaults to `true` (i.e. keep the backup files). +mergetool.prompt:: + Prompt before each invocation of the merge resolution program. + pack.window:: The size of the window used by linkgit:git-pack-objects[1] when no window size is given on the command line. Defaults to 10. diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt index e0b2703..176483a 100644 --- a/Documentation/git-mergetool.txt +++ b/Documentation/git-mergetool.txt @@ -7,7 +7,7 @@ git-mergetool - Run merge conflict resolution tools to resolve merge conflicts SYNOPSIS -------- -'git mergetool' [--tool=<tool>] [<file>]... +'git mergetool' [--tool=<tool>] [-y|--no-prompt|--prompt] [<file>]... DESCRIPTION ----------- @@ -60,6 +60,15 @@ variable `mergetool.<tool>.trustExitCode` can be set to `true`. Otherwise, 'git-mergetool' will prompt the user to indicate the success of the resolution after the custom tool has exited. +-y or --no-prompt:: + Don't prompt before each invocation of the merge resolution + program. + +--prompt:: + Prompt before each invocation of the merge resolution program. + This is the default behaviour; the option is provided to + override any configuration settings. + Author ------ Written by Theodore Y ...
This option stops git mergetool from aborting at the first failed merge. This allows some additional use patterns. Merge conflicts can now be previewed one at time and merges can also be skipped so that they can be performed in a later pass. There is also a mergetool.keepGoing configuration variable covering the same behaviour. Signed-off-by: Charles Bailey <charles@hashpling.org> --- Documentation/config.txt | 4 +++ Documentation/git-mergetool.txt | 12 +++++++++- git-mergetool.sh | 46 ++++++++++++++++++++++++++++++-------- 3 files changed, 51 insertions(+), 11 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index c5b211a..0b0bc99 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -976,6 +976,10 @@ mergetool.keepBackup:: is set to `false` then this file is not preserved. Defaults to `true` (i.e. keep the backup files). +mergetool.keepGoing:: + Continue to attempt resolution of remaining conflicted files even + after a merge has failed or been aborted. + mergetool.prompt:: Prompt before each invocation of the merge resolution program. diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt index 176483a..bd2a5ab 100644 --- a/Documentation/git-mergetool.txt +++ b/Documentation/git-mergetool.txt @@ -7,7 +7,8 @@ git-mergetool - Run merge conflict resolution tools to resolve merge conflicts SYNOPSIS -------- -'git mergetool' [--tool=<tool>] [-y|--no-prompt|--prompt] [<file>]... +'git mergetool' [--tool=<tool>] [-y|--no-prompt|--prompt] + [-k|--keep-going|--no-keep-going] [<file>]... DESCRIPTION ----------- @@ -69,6 +70,15 @@ success of the resolution after the custom tool has exited. This is the default behaviour; the option is provided to override any configuration settings. +-k or --keep-going:: + Continue to attempt resolution of remaining conflicted files + even after a merge has failed or been ...
I think this series addresses the questions I remember from the first This patch in particular changes some of the innards of mergetool, and while the changes look good to me via reading, I would feel even more comfortable if there were some tests (it looks like your previous t7610 gives at least a basic sanity check, but it might be nice to test to One of these is not like the others. Is there a reason to add a cleanup_temp_files here (and if so, should it be noted in the commit message, and/or be in a separate commit?). -Peff --
I had a little difficulty thinking of some decently robust tests. git mergetool is, by its nature, more interactive than many git commands. With --no-prompt it should be easier to make some tests that don't just Ah yes, it's definitely worthy of at least a comment. This is something that I changed right at the end of testing and I should really have made it into a separate commit. Previously, if you aborted a merge, you were left with the base/local/remote temporaries for the merge that you aborted. To be honest, I found this a little irritating. The base, local and remote temporaries are inputs so are accessible from slots 1,2 and 3 of the index, and any intermediate output will be in the target file. You can git clean, but if you have other temporaries you need to keep, you end up having to manually clean them up in any case. With --keep-going, the problem is compounded. If you make several passes through a list of complex merges you end up with fistfuls of these temporary trios in your working tree. It goes from slightly annoying to very irritating. Charles. --
I agree; I have never found the temporary files left by mergetool from a failed merge to be useful, and get a little annoyed that I have to "git clean" them away afterwards. Even without --keep-going, I would often look at and abort a merge several times before resolving it and end up with several copies. But I definitely think that is an issue for a separate patch, and one that needs input from Ted and from other mergetool users. -Peff --
I agree. On occasion it's useful, but more often than not, keeping the temporaries is more annoying than anything else. At the time when I wrote mergetool, it was a pain for me to either do "git ls-files --stage" and then cut and paste the SHA hash, or to type commands like "git cat-file blob :1:long/pathname/from/the/top.c" to look one various staged versions of the file. I'd suggest that this is probably worth turning into an option (-k|--keep-files), and default the answer to deleting the temporaries before mergetool exits. - Ted --
I wonder why this even needs to be an option. If you do not want to resolve all of them, you can limit the amount of work you would do by giving list of paths to work on, can't you? --
I don't know about Charles, but I often don't know that I don't want to merge a file using the graphical tool until I see it in the graphical tool. IOW, I start trying to merge it there and realize the conflict is such that I am better off doing it in my editor with conflict markers. -Peff --
I have to say that since having this in my tree I've benefitted hugely from using a visual diff tool as a merge preview tool. I'm working on a (far from ideal) project where two branches are diverging fast due to differing goals, yet there is a need for an end product with the functionality of both branches in it. This means that I often end up needing to do frequent large merges. Often it is not until I have the merge open in front of me that it becomes obvious that while a particular file needs merging, the best merge strategy will only be obvious after resolving the conflicts in other parts of the system. Often several hours of work will just be four or five invocations of git mergetool until something which compiles emerges. Stopping to view what the next unmerged file path is and pasting it into another mergetool command line seems like an unnecessary distraction. Charles. --
Instead of making this be a command-line and configuration option,
maybe it would be better to prompt the user after an aborted merge,
and give the user the opportunity to continue or abort? i.e., instead
of just saying "merge of foo.c failed" and then exiting, to ask the
user instad something like, "Merge of foo.c failed; continue
attempting to merge the rest of the files? <y>"
I suspect this might be more friendly than Yet Another command-line
option and configuration parameter. What do you think?
- Ted
--
This sounds like a really good alternative interface option. The next time I have a moment (could be a while!) I'll try and make a patch based on this idea. Charles. --
My apologies for not commeting earlier on your patches; I've been a bit overloaded as of late --- way too much conference travel! :-( I can see why this should perhaps be the default for gui-based merge program. The one place where it should perhaps not be the default is if the tool could be text based (i.e., if you are using text-based emacs to do the emerge). So perhaps what we should do is 1) Make the question of whether or not a particular back-end merge program should require the user to hit return first to be configured on a per-gui tool basis. 2) For all of the gui tools default the answer to be to _not_ prompt first. 3) For the emacs-based merge, make the default be based on whether or not $DISPLAY is set. (But allow an overide based on mergetool.emerge.prompt.) I think this much more accurately would model what users want, and if we get the default right for most users, that's definitely a win. Regards, - Ted --