Re: [PATCH v3] Allow the user to change the temporary file name for mergetool

Johannes Sixt [off-list ref] writes:

Am 19.08.2014 19:15, schrieb Robin Rosenberg:

quoted

Using the original filename suffix for the temporary input files to
the merge tool confuses IDEs like Eclipse. This patch introduces
a configurtion option, mergetool.tmpsuffix, which get appended to
the temporary file name. That way the user can choose to use a
suffix like ".tmp", which does not cause confusion.

I have a merge tool that does syntax highlighting based on the file
extension. Given this:

quoted

+	BACKUP="./$MERGED.BACKUP.$ext$tmpsuffix"
+	LOCAL="./$MERGED.LOCAL.$ext$tmpsuffix"
+	REMOTE="./$MERGED.REMOTE.$ext$tmpsuffix"
+	BASE="./$MERGED.BASE.$ext$tmpsuffix"

I guess I lose syntax highlighting if I were to use mergetool.tmpsuffix;
but then I don't use Eclipse. Could it be that this is really just a
band-aid for Eclipse users, not IDEs in general as you are hinting in
the Documentation of the new variable?

The phrase "IDEs like Eclipse" in the proposed log message did not
tell me (which I think is a good thing) if IDEs that need "band-aid"
are majority or minority, but I agree that we should not hint that
IDEs in general would benefit by setting this variable.  A warning
on the syntax-aware editors may be necessary.

Thanks for a careful reading.

----- Ursprungligt meddelande -----

Från: "Junio C Hamano" [off-list ref]
Till: "Johannes Sixt" [off-list ref]
Kopia: "Robin Rosenberg" [off-list ref], git@vger.kernel.org
Skickat: onsdag, 20 aug 2014 0:14:21
Ämne: Re: [PATCH v3] Allow the user to change the temporary file name for mergetool

Johannes Sixt [off-list ref] writes:

quoted

Am 19.08.2014 19:15, schrieb Robin Rosenberg:

quoted

Using the original filename suffix for the temporary input files to
the merge tool confuses IDEs like Eclipse. This patch introduces
a configurtion option, mergetool.tmpsuffix, which get appended to
the temporary file name. That way the user can choose to use a
suffix like ".tmp", which does not cause confusion.

I have a merge tool that does syntax highlighting based on the file
extension. Given this:

quoted

+	BACKUP="./$MERGED.BACKUP.$ext$tmpsuffix"
+	LOCAL="./$MERGED.LOCAL.$ext$tmpsuffix"
+	REMOTE="./$MERGED.REMOTE.$ext$tmpsuffix"
+	BASE="./$MERGED.BASE.$ext$tmpsuffix"

I guess I lose syntax highlighting if I were to use mergetool.tmpsuffix;
but then I don't use Eclipse. Could it be that this is really just a
band-aid for Eclipse users, not IDEs in general as you are hinting in
the Documentation of the new variable?

The phrase "IDEs like Eclipse" in the proposed log message did not
tell me (which I think is a good thing) if IDEs that need "band-aid"
are majority or minority, but I agree that we should not hint that
IDEs in general would benefit by setting this variable.  A warning
on the syntax-aware editors may be necessary.

I'm not sure it's necessary since it is not a default. If you use the
setting you are probably well aware of why you use it, and the 
possible implications.

I have only had the problem with Eclipse, but I imagine any tool that
"owns" a directory and scans it for changes will find these temporary
files and do something unexpected based on the suffix. By setting the
suffix to something insert, like txt, tmp, dat or whatever you prevent
that tool from thinking too much.

In concrete terms, what happens is that Eclipse, in my case, find
temporary filenames with the suffix Foo.REMOTE.java and thinks that
is the one source file for Foo since it contains the source for Foo.

Sure you lose syntax highlighting, that's a trade-off. An alternative
solution would be to put these files somewhere else.

-- robin

Using the original filename suffix for the temporary input files to
the merge tool confuses IDEs like Eclipse. This patch introduces
a configurtion option, mergetool.tmpsuffix, which get appended to
the temporary file name. That way the user can choose to use a
suffix like ".tmp", which does not cause confusion.

Signed-off-by: Robin Rosenberg <redacted>
---
 Documentation/config.txt        |  5 +++++
 Documentation/git-mergetool.txt |  7 +++++++
 git-mergetool.sh                | 10 ++++++----
 3 files changed, 18 insertions(+), 4 deletions(-)

Fixed a spelling error.

-- robin

diff --git a/Documentation/config.txt b/Documentation/config.txt
index c55c22a..0e15800 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt

@@ -1778,6 +1778,11 @@ notes.displayRef::
 	several times.  A warning will be issued for refs that do not
 	exist, but a glob that does not match any refs is silently
 	ignored.
+
+mergetool.tmpsuffix::
+	A string to append the names of the temporary files mergetool
+	creates in the worktree as input to a custom merge tool. The
+	primary use is to avoid confusion in IDEs during merge.
 +
 This setting can be overridden with the `GIT_NOTES_DISPLAY_REF`
 environment variable, which must be a colon separated list of refs or

diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index e846c2e..80a0526 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt

@@ -89,6 +89,13 @@ Setting the `mergetool.keepBackup` configuration variable to `false`
 causes `git mergetool` to automatically remove the backup as files
 are successfully merged.
 
+`git mergetool` may also create other temporary files for the
+different versions involved in the merge. By default these files have
+the same filename suffix as the file being merged. This may confuse
+other tools in use during a long merge operation. The user can set
+`mergetool.tmpsuffix` to be used as an extra suffix, which will be
+appened to the temporary filename to lessen that problem.
+
 GIT
 ---
 Part of the linkgit:git[1] suite

diff --git a/git-mergetool.sh b/git-mergetool.sh
index 9a046b7..d7cc76c 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh

@@ -214,6 +214,8 @@ checkout_staged_file () {
 }
 
 merge_file () {
+	tmpsuffix=$(git config mergetool.tmpsuffix || true)
+
 	MERGED="$1"
 
 	f=$(git ls-files -u -- "$MERGED")

@@ -229,10 +231,10 @@ merge_file () {
 	fi
 
 	ext="$$$(expr "$MERGED" : '.*\(\.[^/]*\)$')"
-	BACKUP="./$MERGED.BACKUP.$ext"
-	LOCAL="./$MERGED.LOCAL.$ext"
-	REMOTE="./$MERGED.REMOTE.$ext"
-	BASE="./$MERGED.BASE.$ext"
+	BACKUP="./$MERGED.BACKUP.$ext$tmpsuffix"
+	LOCAL="./$MERGED.LOCAL.$ext$tmpsuffix"
+	REMOTE="./$MERGED.REMOTE.$ext$tmpsuffix"
+	BASE="./$MERGED.BASE.$ext$tmpsuffix"
 
 	base_mode=$(git ls-files -u -- "$MERGED" | awk '{if ($3==1) print $1;}')
 	local_mode=$(git ls-files -u -- "$MERGED" | awk '{if ($3==2) print $1;}')

-- 
2.1.0.rc2.6.g39c33ff.dirty