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

2014-08-20 Thread Stefan Näwe
Am 19.08.2014 um 19:15 schrieb Robin Rosenberg:
 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 robin.rosenb...@dewire.com
 ---
  Documentation/config.txt|  5 +
  Documentation/git-mergetool.txt |  7 +++
  git-mergetool.sh| 10 ++
  3 files changed, 18 insertions(+), 4 deletions(-)
 
 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.

Still:

s/appened/appended/

 +
  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;}')
 

Stefan
-- 

/dev/random says: It's Ensign Flintstone, Jim... He's Fred!
python -c print 
'73746566616e2e6e616577654061746c61732d656c656b74726f6e696b2e636f6d'.decode('hex')
 
GPG Key fingerprint = 2DF5 E01B 09C3 7501 BCA9  9666 829B 49C5 9221 27AF
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-08-20 Thread Robin Rosenberg


- Ursprungligt meddelande -
 Från: Junio C Hamano gits...@pobox.com
 Till: Johannes Sixt j...@kdbg.org
 Kopia: Robin Rosenberg robin.rosenb...@dewire.com, 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 j...@kdbg.org writes:
 
  Am 19.08.2014 19:15, schrieb Robin Rosenberg:
  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:
 
  +  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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-08-19 Thread Robin Rosenberg
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 robin.rosenb...@dewire.com
---
 Documentation/config.txt|  5 +
 Documentation/git-mergetool.txt |  7 +++
 git-mergetool.sh| 10 ++
 3 files changed, 18 insertions(+), 4 deletions(-)

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

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-08-19 Thread Junio C Hamano
Robin Rosenberg robin.rosenb...@dewire.com writes:

 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 robin.rosenb...@dewire.com
 ---
  Documentation/config.txt|  5 +
  Documentation/git-mergetool.txt |  7 +++
  git-mergetool.sh| 10 ++
  3 files changed, 18 insertions(+), 4 deletions(-)

Thanks for a quick turn-around.


 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

I smell that the new paragraph is inserted at a wrong place.  What
does notes-display-ref environment have anything to do with this
variable?

Also, could you phrase this in a way to hint that the users are
likely to want to begin the value for this variable with a dot (or
some other special character)?  Your 'suffix like .tmp' in the
proposed log message does it very nicely [*1*], and I'd like to see the
same done for the end users who do not have access to our log
message but do have access to our documentation pages.

Same comment applies to the new paragraph in the documentation of
git-mergetool itself.

 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.
 +

[Footnote]

*1* To anybody remotely intelligent (like me), it hints that a
temporary file would have a name like hello.rbtmp if it is set to
tmp, to let them make a natural inference that they are better off
using something like .tmp, ~tmp, +tmp, etc.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-08-19 Thread Johannes Sixt
Am 19.08.2014 19:15, schrieb Robin Rosenberg:
 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:

 + 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?

-- Hannes

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-08-19 Thread Junio C Hamano
Johannes Sixt j...@kdbg.org writes:

 Am 19.08.2014 19:15, schrieb Robin Rosenberg:
 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:

 +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.



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html