Re: [PATCH v3] Allow the user to change the temporary file name for mergetool
- Ursprungligt meddelande - > Från: "Junio C Hamano" > Till: "Johannes Sixt" > Kopia: "Robin Rosenberg" , 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 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
Re: [PATCH v3] Allow the user to change the temporary file name for mergetool
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
> ---
> 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
Johannes Sixt 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
Re: [PATCH v3] Allow the user to change the temporary file name for mergetool
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
Robin Rosenberg 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 > --- > 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
[PATCH v3] Allow the user to change the temporary file name for mergetool
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
---
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
