* Using clean/smudge filters with difftool
@ 2015-06-16 14:11 Florian Aspart
2015-06-18 12:31 ` Michael J Gruber
0 siblings, 1 reply; 18+ messages in thread
From: Florian Aspart @ 2015-06-16 14:11 UTC (permalink / raw)
To: git
Hi everyone,
I created a clean filter to apply on some files before commiting them.
The filter works correctly when I commit the file and is also applied
when I usethe iff command line tool.
However, when using difftool with meld, the filter is not applied and
the different versions of the files are compared without any
filtering.
Is there a way to apply the clean/smudge filters when comparing the
working copy of a file to the HEAD version in a gui diff tool?
I'm using git version 2.4.3 under Ubuntu.
Best,
Florian
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Using clean/smudge filters with difftool
2015-06-16 14:11 Using clean/smudge filters with difftool Florian Aspart
@ 2015-06-18 12:31 ` Michael J Gruber
2015-06-18 13:15 ` Florian Aspart
0 siblings, 1 reply; 18+ messages in thread
From: Michael J Gruber @ 2015-06-18 12:31 UTC (permalink / raw)
To: Florian Aspart, git
Florian Aspart venit, vidit, dixit 16.06.2015 16:11:
> Hi everyone,
>
> I created a clean filter to apply on some files before commiting them.
> The filter works correctly when I commit the file and is also applied
> when I usethe iff command line tool.
> However, when using difftool with meld, the filter is not applied and
> the different versions of the files are compared without any
> filtering.
>
> Is there a way to apply the clean/smudge filters when comparing the
> working copy of a file to the HEAD version in a gui diff tool?
>
> I'm using git version 2.4.3 under Ubuntu.
>
> Best,
> Florian
Are you saying that "difftool" compares an uncleaned working tree file
with a cleaned blob? That would be a bug in either difftool or the way
we feed difftool.
Michael
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Using clean/smudge filters with difftool
2015-06-18 12:31 ` Michael J Gruber
@ 2015-06-18 13:15 ` Florian Aspart
2015-06-18 13:26 ` John Keeping
0 siblings, 1 reply; 18+ messages in thread
From: Florian Aspart @ 2015-06-18 13:15 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git
Hi Michael,
yes in this case "difftool" compares an uncleaned working tree file
with a cleaned blob. I did not try the smudge filter to see if it
applied in difftool.
I think the problem comes from the way difftool is feeded, since I
also had this problem when setting an external tool for the diff in
the gitconfig file.
However, I'm not sure if this is a bug or it is designed to be so.
If the external tool changes a cleaned working tree file during the
diff, then by saving this file the result of the cleaning filter would
also be saved in the working tree.
2015-06-18 14:31 GMT+02:00 Michael J Gruber <git@drmicha.warpmail.net>:
> Florian Aspart venit, vidit, dixit 16.06.2015 16:11:
>> Hi everyone,
>>
>> I created a clean filter to apply on some files before commiting them.
>> The filter works correctly when I commit the file and is also applied
>> when I usethe iff command line tool.
>> However, when using difftool with meld, the filter is not applied and
>> the different versions of the files are compared without any
>> filtering.
>>
>> Is there a way to apply the clean/smudge filters when comparing the
>> working copy of a file to the HEAD version in a gui diff tool?
>>
>> I'm using git version 2.4.3 under Ubuntu.
>>
>> Best,
>> Florian
>
> Are you saying that "difftool" compares an uncleaned working tree file
> with a cleaned blob? That would be a bug in either difftool or the way
> we feed difftool.
>
> Michael
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Using clean/smudge filters with difftool
2015-06-18 13:15 ` Florian Aspart
@ 2015-06-18 13:26 ` John Keeping
2015-06-18 13:51 ` Florian Aspart
0 siblings, 1 reply; 18+ messages in thread
From: John Keeping @ 2015-06-18 13:26 UTC (permalink / raw)
To: Florian Aspart; +Cc: Michael J Gruber, git
[Please don't top-post on this list.]
On Thu, Jun 18, 2015 at 03:15:38PM +0200, Florian Aspart wrote:
> 2015-06-18 14:31 GMT+02:00 Michael J Gruber <git@drmicha.warpmail.net>:
> > Florian Aspart venit, vidit, dixit 16.06.2015 16:11:
> >> Hi everyone,
> >>
> >> I created a clean filter to apply on some files before commiting them.
> >> The filter works correctly when I commit the file and is also applied
> >> when I usethe iff command line tool.
> >> However, when using difftool with meld, the filter is not applied and
> >> the different versions of the files are compared without any
> >> filtering.
> >>
> >> Is there a way to apply the clean/smudge filters when comparing the
> >> working copy of a file to the HEAD version in a gui diff tool?
> >>
> >> I'm using git version 2.4.3 under Ubuntu.
> >>
> >> Best,
> >> Florian
> >
> > Are you saying that "difftool" compares an uncleaned working tree file
> > with a cleaned blob? That would be a bug in either difftool or the way
> > we feed difftool.
> >
> yes in this case "difftool" compares an uncleaned working tree file
> with a cleaned blob. I did not try the smudge filter to see if it
> applied in difftool.
>
> I think the problem comes from the way difftool is feeded, since I
> also had this problem when setting an external tool for the diff in
> the gitconfig file.
>
> However, I'm not sure if this is a bug or it is designed to be so.
> If the external tool changes a cleaned working tree file during the
> diff, then by saving this file the result of the cleaning filter would
> also be saved in the working tree.
How is your filter configured? Is it using a simple pattern (e.g.
"*.c") or is it using a file path?
git-difftool uses `git checkout-index --all --prefix=$dir/` and I wonder
if the prefix means that the attribute specification does not match the
temporary file that difftool produces, so no filter is applied.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Using clean/smudge filters with difftool
2015-06-18 13:26 ` John Keeping
@ 2015-06-18 13:51 ` Florian Aspart
2015-06-18 14:11 ` John Keeping
0 siblings, 1 reply; 18+ messages in thread
From: Florian Aspart @ 2015-06-18 13:51 UTC (permalink / raw)
To: John Keeping; +Cc: Michael J Gruber, git
2015-06-18 15:26 GMT+02:00 John Keeping <john@keeping.me.uk>:
> [Please don't top-post on this list.]
>
> On Thu, Jun 18, 2015 at 03:15:38PM +0200, Florian Aspart wrote:
>> 2015-06-18 14:31 GMT+02:00 Michael J Gruber <git@drmicha.warpmail.net>:
>> > Florian Aspart venit, vidit, dixit 16.06.2015 16:11:
>> >> Hi everyone,
>> >>
>> >> I created a clean filter to apply on some files before commiting them.
>> >> The filter works correctly when I commit the file and is also applied
>> >> when I usethe iff command line tool.
>> >> However, when using difftool with meld, the filter is not applied and
>> >> the different versions of the files are compared without any
>> >> filtering.
>> >>
>> >> Is there a way to apply the clean/smudge filters when comparing the
>> >> working copy of a file to the HEAD version in a gui diff tool?
>> >>
>> >> I'm using git version 2.4.3 under Ubuntu.
>> >>
>> >> Best,
>> >> Florian
>> >
>> > Are you saying that "difftool" compares an uncleaned working tree file
>> > with a cleaned blob? That would be a bug in either difftool or the way
>> > we feed difftool.
>> >
>> yes in this case "difftool" compares an uncleaned working tree file
>> with a cleaned blob. I did not try the smudge filter to see if it
>> applied in difftool.
>>
>> I think the problem comes from the way difftool is feeded, since I
>> also had this problem when setting an external tool for the diff in
>> the gitconfig file.
>>
>> However, I'm not sure if this is a bug or it is designed to be so.
>> If the external tool changes a cleaned working tree file during the
>> diff, then by saving this file the result of the cleaning filter would
>> also be saved in the working tree.
>
> How is your filter configured? Is it using a simple pattern (e.g.
> "*.c") or is it using a file path?
>
> git-difftool uses `git checkout-index --all --prefix=$dir/` and I wonder
> if the prefix means that the attribute specification does not match the
> temporary file that difftool produces, so no filter is applied.
It is using a simple pattern:
*.ipynb filter=clean_ipynb
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Using clean/smudge filters with difftool
2015-06-18 13:51 ` Florian Aspart
@ 2015-06-18 14:11 ` John Keeping
2015-06-18 14:17 ` Florian Aspart
0 siblings, 1 reply; 18+ messages in thread
From: John Keeping @ 2015-06-18 14:11 UTC (permalink / raw)
To: Florian Aspart; +Cc: Michael J Gruber, git
On Thu, Jun 18, 2015 at 03:51:25PM +0200, Florian Aspart wrote:
> 2015-06-18 15:26 GMT+02:00 John Keeping <john@keeping.me.uk>:
> > [Please don't top-post on this list.]
> >
> > On Thu, Jun 18, 2015 at 03:15:38PM +0200, Florian Aspart wrote:
> >> 2015-06-18 14:31 GMT+02:00 Michael J Gruber <git@drmicha.warpmail.net>:
> >> > Florian Aspart venit, vidit, dixit 16.06.2015 16:11:
> >> >> Hi everyone,
> >> >>
> >> >> I created a clean filter to apply on some files before commiting them.
> >> >> The filter works correctly when I commit the file and is also applied
> >> >> when I usethe iff command line tool.
> >> >> However, when using difftool with meld, the filter is not applied and
> >> >> the different versions of the files are compared without any
> >> >> filtering.
> >> >>
> >> >> Is there a way to apply the clean/smudge filters when comparing the
> >> >> working copy of a file to the HEAD version in a gui diff tool?
> >> >>
> >> >> I'm using git version 2.4.3 under Ubuntu.
> >> >>
> >> >> Best,
> >> >> Florian
> >> >
> >> > Are you saying that "difftool" compares an uncleaned working tree file
> >> > with a cleaned blob? That would be a bug in either difftool or the way
> >> > we feed difftool.
> >> >
> >> yes in this case "difftool" compares an uncleaned working tree file
> >> with a cleaned blob. I did not try the smudge filter to see if it
> >> applied in difftool.
> >>
> >> I think the problem comes from the way difftool is feeded, since I
> >> also had this problem when setting an external tool for the diff in
> >> the gitconfig file.
> >>
> >> However, I'm not sure if this is a bug or it is designed to be so.
> >> If the external tool changes a cleaned working tree file during the
> >> diff, then by saving this file the result of the cleaning filter would
> >> also be saved in the working tree.
> >
> > How is your filter configured? Is it using a simple pattern (e.g.
> > "*.c") or is it using a file path?
> >
> > git-difftool uses `git checkout-index --all --prefix=$dir/` and I wonder
> > if the prefix means that the attribute specification does not match the
> > temporary file that difftool produces, so no filter is applied.
>
> It is using a simple pattern:
> *.ipynb filter=clean_ipynb
I also realised that the code for file diff is very different from
directory diff do you see any difference between git-difftool acting on
files and with the `--dir-diff` option?
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Using clean/smudge filters with difftool
2015-06-18 14:11 ` John Keeping
@ 2015-06-18 14:17 ` Florian Aspart
2015-06-18 14:28 ` John Keeping
0 siblings, 1 reply; 18+ messages in thread
From: Florian Aspart @ 2015-06-18 14:17 UTC (permalink / raw)
To: John Keeping; +Cc: Michael J Gruber, git
2015-06-18 16:11 GMT+02:00 John Keeping <john@keeping.me.uk>:
> On Thu, Jun 18, 2015 at 03:51:25PM +0200, Florian Aspart wrote:
>> 2015-06-18 15:26 GMT+02:00 John Keeping <john@keeping.me.uk>:
>> > [Please don't top-post on this list.]
>> >
>> > On Thu, Jun 18, 2015 at 03:15:38PM +0200, Florian Aspart wrote:
>> >> 2015-06-18 14:31 GMT+02:00 Michael J Gruber <git@drmicha.warpmail.net>:
>> >> > Florian Aspart venit, vidit, dixit 16.06.2015 16:11:
>> >> >> Hi everyone,
>> >> >>
>> >> >> I created a clean filter to apply on some files before commiting them.
>> >> >> The filter works correctly when I commit the file and is also applied
>> >> >> when I usethe iff command line tool.
>> >> >> However, when using difftool with meld, the filter is not applied and
>> >> >> the different versions of the files are compared without any
>> >> >> filtering.
>> >> >>
>> >> >> Is there a way to apply the clean/smudge filters when comparing the
>> >> >> working copy of a file to the HEAD version in a gui diff tool?
>> >> >>
>> >> >> I'm using git version 2.4.3 under Ubuntu.
>> >> >>
>> >> >> Best,
>> >> >> Florian
>> >> >
>> >> > Are you saying that "difftool" compares an uncleaned working tree file
>> >> > with a cleaned blob? That would be a bug in either difftool or the way
>> >> > we feed difftool.
>> >> >
>> >> yes in this case "difftool" compares an uncleaned working tree file
>> >> with a cleaned blob. I did not try the smudge filter to see if it
>> >> applied in difftool.
>> >>
>> >> I think the problem comes from the way difftool is feeded, since I
>> >> also had this problem when setting an external tool for the diff in
>> >> the gitconfig file.
>> >>
>> >> However, I'm not sure if this is a bug or it is designed to be so.
>> >> If the external tool changes a cleaned working tree file during the
>> >> diff, then by saving this file the result of the cleaning filter would
>> >> also be saved in the working tree.
>> >
>> > How is your filter configured? Is it using a simple pattern (e.g.
>> > "*.c") or is it using a file path?
>> >
>> > git-difftool uses `git checkout-index --all --prefix=$dir/` and I wonder
>> > if the prefix means that the attribute specification does not match the
>> > temporary file that difftool produces, so no filter is applied.
>>
>> It is using a simple pattern:
>> *.ipynb filter=clean_ipynb
>
> I also realised that the code for file diff is very different from
> directory diff do you see any difference between git-difftool acting on
> files and with the `--dir-diff` option?
No, even with the --dir-diff option, the filter is still not applied.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Using clean/smudge filters with difftool
2015-06-18 14:17 ` Florian Aspart
@ 2015-06-18 14:28 ` John Keeping
2015-06-18 15:39 ` Florian Aspart
0 siblings, 1 reply; 18+ messages in thread
From: John Keeping @ 2015-06-18 14:28 UTC (permalink / raw)
To: Florian Aspart; +Cc: Michael J Gruber, git
On Thu, Jun 18, 2015 at 04:17:52PM +0200, Florian Aspart wrote:
> 2015-06-18 16:11 GMT+02:00 John Keeping <john@keeping.me.uk>:
> > On Thu, Jun 18, 2015 at 03:51:25PM +0200, Florian Aspart wrote:
> >> 2015-06-18 15:26 GMT+02:00 John Keeping <john@keeping.me.uk>:
> >> > [Please don't top-post on this list.]
> >> >
> >> > On Thu, Jun 18, 2015 at 03:15:38PM +0200, Florian Aspart wrote:
> >> >> 2015-06-18 14:31 GMT+02:00 Michael J Gruber <git@drmicha.warpmail.net>:
> >> >> > Florian Aspart venit, vidit, dixit 16.06.2015 16:11:
> >> >> >> I created a clean filter to apply on some files before commiting them.
> >> >> >> The filter works correctly when I commit the file and is also applied
> >> >> >> when I usethe iff command line tool.
> >> >> >> However, when using difftool with meld, the filter is not applied and
> >> >> >> the different versions of the files are compared without any
> >> >> >> filtering.
> >> >> >>
> >> >> >> Is there a way to apply the clean/smudge filters when comparing the
> >> >> >> working copy of a file to the HEAD version in a gui diff tool?
> >> >> >>
> >> >> >> I'm using git version 2.4.3 under Ubuntu.
> >
> > I also realised that the code for file diff is very different from
> > directory diff do you see any difference between git-difftool acting on
> > files and with the `--dir-diff` option?
>
> No, even with the --dir-diff option, the filter is still not applied.
I have tried to reproduce this and it works as expected for me (i.e. the
filter is applied) both for file diff and directory diff mode:
$ git config filter.quote.clean "sed -e 's/^> //'"
$ git config filter.quote.smudge "sed -e '/^> /n; s/^/> /'"
$ git config filter.quote.required true
$ echo '*.quote filter=quote' >>.gitattributes
$ cat >1.quote <<EOF
one
two
three
EOF
$ git add .gitattributes 1.quote
$ git commit -m 'Initial commit'
$ echo four >>1.quote
Now `git-difftool` shows the differences with the filter applied. This can be
seen running with GIT_TRACE:
$ GIT_TRACE=2 git difftool
15:26:59.211541 git.c:557 trace: exec: 'git-difftool'
15:26:59.211674 run-command.c:347 trace: run_command: 'git-difftool'
15:26:59.338617 git.c:348 trace: built-in: git 'config' '--bool' '--get' 'difftool.trustExitCode'
15:26:59.342664 git.c:348 trace: built-in: git 'diff'
15:26:59.344857 run-command.c:347 trace: run_command: 'sed -e '\''s/^> //'\'''
15:26:59.345383 run-command.c:195 trace: exec: '/bin/sh' '-c' 'sed -e '\''s/^> //'\''' 'sed -e '\''s/^> //'\'''
15:26:59.351077 run-command.c:347 trace: run_command: 'sed -e '\''/^> /n; s/^/> /'\'''
15:26:59.351605 run-command.c:195 trace: exec: '/bin/sh' '-c' 'sed -e '\''/^> /n; s/^/> /'\''' 'sed -e '\''/^> /n; s/^/> /'\'''
15:26:59.355716 run-command.c:347 trace: run_command: 'git-difftool--helper' '1.quote' '/tmp/SUEySx_1.quote' '4cb29ea38f70d7c61b2a3a25b02e3bdf44905402' '100644' '1.quote' '0000000000000000000000000000000000000000' '100644'
15:26:59.356191 run-command.c:195 trace: exec: 'git-difftool--helper' '1.quote' '/tmp/SUEySx_1.quote' '4cb29ea38f70d7c61b2a3a25b02e3bdf44905402' '100644' '1.quote' '0000000000000000000000000000000000000000' '100644'
15:26:59.370468 git.c:348 trace: built-in: git 'config' 'diff.tool'
15:26:59.373485 git.c:348 trace: built-in: git 'config' 'merge.tool'
15:26:59.378402 git.c:348 trace: built-in: git 'config' 'difftool.vimdiff.cmd'
15:26:59.381424 git.c:348 trace: built-in: git 'config' 'mergetool.vimdiff.cmd'
15:26:59.386623 git.c:348 trace: built-in: git 'config' '--bool' 'mergetool.prompt'
15:26:59.390198 git.c:348 trace: built-in: git 'config' '--bool' 'difftool.prompt'
I think the first run_command of `sed` is cleaning the working tree file
to figure out *if* it differs, then the second `sed` is smudging the
version in the index so that difftool can use it.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Using clean/smudge filters with difftool
2015-06-18 14:28 ` John Keeping
@ 2015-06-18 15:39 ` Florian Aspart
2015-06-18 16:01 ` John Keeping
0 siblings, 1 reply; 18+ messages in thread
From: Florian Aspart @ 2015-06-18 15:39 UTC (permalink / raw)
To: John Keeping; +Cc: Michael J Gruber, git
2015-06-18 16:28 GMT+02:00 John Keeping <john@keeping.me.uk>:
> On Thu, Jun 18, 2015 at 04:17:52PM +0200, Florian Aspart wrote:
>> 2015-06-18 16:11 GMT+02:00 John Keeping <john@keeping.me.uk>:
>> > On Thu, Jun 18, 2015 at 03:51:25PM +0200, Florian Aspart wrote:
>> >> 2015-06-18 15:26 GMT+02:00 John Keeping <john@keeping.me.uk>:
>> >> > [Please don't top-post on this list.]
>> >> >
>> >> > On Thu, Jun 18, 2015 at 03:15:38PM +0200, Florian Aspart wrote:
>> >> >> 2015-06-18 14:31 GMT+02:00 Michael J Gruber <git@drmicha.warpmail.net>:
>> >> >> > Florian Aspart venit, vidit, dixit 16.06.2015 16:11:
>> >> >> >> I created a clean filter to apply on some files before commiting them.
>> >> >> >> The filter works correctly when I commit the file and is also applied
>> >> >> >> when I usethe iff command line tool.
>> >> >> >> However, when using difftool with meld, the filter is not applied and
>> >> >> >> the different versions of the files are compared without any
>> >> >> >> filtering.
>> >> >> >>
>> >> >> >> Is there a way to apply the clean/smudge filters when comparing the
>> >> >> >> working copy of a file to the HEAD version in a gui diff tool?
>> >> >> >>
>> >> >> >> I'm using git version 2.4.3 under Ubuntu.
>> >
>> > I also realised that the code for file diff is very different from
>> > directory diff do you see any difference between git-difftool acting on
>> > files and with the `--dir-diff` option?
>>
>> No, even with the --dir-diff option, the filter is still not applied.
>
> I have tried to reproduce this and it works as expected for me (i.e. the
> filter is applied) both for file diff and directory diff mode:
>
> $ git config filter.quote.clean "sed -e 's/^> //'"
> $ git config filter.quote.smudge "sed -e '/^> /n; s/^/> /'"
> $ git config filter.quote.required true
>
> $ echo '*.quote filter=quote' >>.gitattributes
> $ cat >1.quote <<EOF
> one
> two
> three
> EOF
> $ git add .gitattributes 1.quote
> $ git commit -m 'Initial commit'
> $ echo four >>1.quote
>
> Now `git-difftool` shows the differences with the filter applied. This can be
> seen running with GIT_TRACE:
>
> $ GIT_TRACE=2 git difftool
> 15:26:59.211541 git.c:557 trace: exec: 'git-difftool'
> 15:26:59.211674 run-command.c:347 trace: run_command: 'git-difftool'
> 15:26:59.338617 git.c:348 trace: built-in: git 'config' '--bool' '--get' 'difftool.trustExitCode'
> 15:26:59.342664 git.c:348 trace: built-in: git 'diff'
> 15:26:59.344857 run-command.c:347 trace: run_command: 'sed -e '\''s/^> //'\'''
> 15:26:59.345383 run-command.c:195 trace: exec: '/bin/sh' '-c' 'sed -e '\''s/^> //'\''' 'sed -e '\''s/^> //'\'''
> 15:26:59.351077 run-command.c:347 trace: run_command: 'sed -e '\''/^> /n; s/^/> /'\'''
> 15:26:59.351605 run-command.c:195 trace: exec: '/bin/sh' '-c' 'sed -e '\''/^> /n; s/^/> /'\''' 'sed -e '\''/^> /n; s/^/> /'\'''
> 15:26:59.355716 run-command.c:347 trace: run_command: 'git-difftool--helper' '1.quote' '/tmp/SUEySx_1.quote' '4cb29ea38f70d7c61b2a3a25b02e3bdf44905402' '100644' '1.quote' '0000000000000000000000000000000000000000' '100644'
> 15:26:59.356191 run-command.c:195 trace: exec: 'git-difftool--helper' '1.quote' '/tmp/SUEySx_1.quote' '4cb29ea38f70d7c61b2a3a25b02e3bdf44905402' '100644' '1.quote' '0000000000000000000000000000000000000000' '100644'
> 15:26:59.370468 git.c:348 trace: built-in: git 'config' 'diff.tool'
> 15:26:59.373485 git.c:348 trace: built-in: git 'config' 'merge.tool'
> 15:26:59.378402 git.c:348 trace: built-in: git 'config' 'difftool.vimdiff.cmd'
> 15:26:59.381424 git.c:348 trace: built-in: git 'config' 'mergetool.vimdiff.cmd'
> 15:26:59.386623 git.c:348 trace: built-in: git 'config' '--bool' 'mergetool.prompt'
> 15:26:59.390198 git.c:348 trace: built-in: git 'config' '--bool' 'difftool.prompt'
>
> I think the first run_command of `sed` is cleaning the working tree file
> to figure out *if* it differs, then the second `sed` is smudging the
> version in the index so that difftool can use it.
I'm not really understanding what your filter is doing, but I tried
your code on my machine and I get a different result when using diff
and difftool on my machine.
The diff results give me:
diff --git a/1.quote b/1.quote
index 4cb29ea..f384549 100644
--- a/1.quote
+++ b/1.quote
@@ -1,3 +1,4 @@
one
two
three
+four
While the diff tool tells me that the repository file is:
> one
> two
> three
and my working copy:
one
two
three
four
In both case the the filters are called twice (cf GIT_TRACE) as in the
example your wrote.
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: Using clean/smudge filters with difftool
2015-06-18 15:39 ` Florian Aspart
@ 2015-06-18 16:01 ` John Keeping
2015-06-18 20:00 ` Junio C Hamano
0 siblings, 1 reply; 18+ messages in thread
From: John Keeping @ 2015-06-18 16:01 UTC (permalink / raw)
To: Florian Aspart; +Cc: Michael J Gruber, git
On Thu, Jun 18, 2015 at 05:39:18PM +0200, Florian Aspart wrote:
> 2015-06-18 16:28 GMT+02:00 John Keeping <john@keeping.me.uk>:
> > On Thu, Jun 18, 2015 at 04:17:52PM +0200, Florian Aspart wrote:
> >> 2015-06-18 16:11 GMT+02:00 John Keeping <john@keeping.me.uk>:
> >> > On Thu, Jun 18, 2015 at 03:51:25PM +0200, Florian Aspart wrote:
> >> >> 2015-06-18 15:26 GMT+02:00 John Keeping <john@keeping.me.uk>:
> >> >> > [Please don't top-post on this list.]
> >> >> >
> >> >> > On Thu, Jun 18, 2015 at 03:15:38PM +0200, Florian Aspart wrote:
> >> >> >> 2015-06-18 14:31 GMT+02:00 Michael J Gruber <git@drmicha.warpmail.net>:
> >> >> >> > Florian Aspart venit, vidit, dixit 16.06.2015 16:11:
> >> >> >> >> I created a clean filter to apply on some files before commiting them.
> >> >> >> >> The filter works correctly when I commit the file and is also applied
> >> >> >> >> when I usethe iff command line tool.
> >> >> >> >> However, when using difftool with meld, the filter is not applied and
> >> >> >> >> the different versions of the files are compared without any
> >> >> >> >> filtering.
> >> >> >> >>
> >> >> >> >> Is there a way to apply the clean/smudge filters when comparing the
> >> >> >> >> working copy of a file to the HEAD version in a gui diff tool?
> >> >> >> >>
> >> >> >> >> I'm using git version 2.4.3 under Ubuntu.
> >> >
> >> > I also realised that the code for file diff is very different from
> >> > directory diff do you see any difference between git-difftool acting on
> >> > files and with the `--dir-diff` option?
> >>
> >> No, even with the --dir-diff option, the filter is still not applied.
> >
> > I have tried to reproduce this and it works as expected for me (i.e. the
> > filter is applied) both for file diff and directory diff mode:
> >
> > $ git config filter.quote.clean "sed -e 's/^> //'"
> > $ git config filter.quote.smudge "sed -e '/^> /n; s/^/> /'"
> > $ git config filter.quote.required true
> >
> > $ echo '*.quote filter=quote' >>.gitattributes
> > $ cat >1.quote <<EOF
> > one
> > two
> > three
> > EOF
> > $ git add .gitattributes 1.quote
> > $ git commit -m 'Initial commit'
> > $ echo four >>1.quote
> >
> > Now `git-difftool` shows the differences with the filter applied. This can be
> > seen running with GIT_TRACE:
> >
> > $ GIT_TRACE=2 git difftool
> > 15:26:59.211541 git.c:557 trace: exec: 'git-difftool'
> > 15:26:59.211674 run-command.c:347 trace: run_command: 'git-difftool'
> > 15:26:59.338617 git.c:348 trace: built-in: git 'config' '--bool' '--get' 'difftool.trustExitCode'
> > 15:26:59.342664 git.c:348 trace: built-in: git 'diff'
> > 15:26:59.344857 run-command.c:347 trace: run_command: 'sed -e '\''s/^> //'\'''
> > 15:26:59.345383 run-command.c:195 trace: exec: '/bin/sh' '-c' 'sed -e '\''s/^> //'\''' 'sed -e '\''s/^> //'\'''
> > 15:26:59.351077 run-command.c:347 trace: run_command: 'sed -e '\''/^> /n; s/^/> /'\'''
> > 15:26:59.351605 run-command.c:195 trace: exec: '/bin/sh' '-c' 'sed -e '\''/^> /n; s/^/> /'\''' 'sed -e '\''/^> /n; s/^/> /'\'''
> > 15:26:59.355716 run-command.c:347 trace: run_command: 'git-difftool--helper' '1.quote' '/tmp/SUEySx_1.quote' '4cb29ea38f70d7c61b2a3a25b02e3bdf44905402' '100644' '1.quote' '0000000000000000000000000000000000000000' '100644'
> > 15:26:59.356191 run-command.c:195 trace: exec: 'git-difftool--helper' '1.quote' '/tmp/SUEySx_1.quote' '4cb29ea38f70d7c61b2a3a25b02e3bdf44905402' '100644' '1.quote' '0000000000000000000000000000000000000000' '100644'
> > 15:26:59.370468 git.c:348 trace: built-in: git 'config' 'diff.tool'
> > 15:26:59.373485 git.c:348 trace: built-in: git 'config' 'merge.tool'
> > 15:26:59.378402 git.c:348 trace: built-in: git 'config' 'difftool.vimdiff.cmd'
> > 15:26:59.381424 git.c:348 trace: built-in: git 'config' 'mergetool.vimdiff.cmd'
> > 15:26:59.386623 git.c:348 trace: built-in: git 'config' '--bool' 'mergetool.prompt'
> > 15:26:59.390198 git.c:348 trace: built-in: git 'config' '--bool' 'difftool.prompt'
> >
> > I think the first run_command of `sed` is cleaning the working tree file
> > to figure out *if* it differs, then the second `sed` is smudging the
> > version in the index so that difftool can use it.
>
> I'm not really understanding what your filter is doing, but I tried
> your code on my machine and I get a different result when using diff
> and difftool on my machine.
It's supposed to be adding "> " to the beginning of lines in the working
tree and stripping them in the repo, but I've just realised that the
instructions above are wrong since "1.quote" is supposed to have leading
"> "'s (although the test I ran before tidying it up for the email was
correct).
> The diff results give me:
>
> diff --git a/1.quote b/1.quote
> index 4cb29ea..f384549 100644
> --- a/1.quote
> +++ b/1.quote
> @@ -1,3 +1,4 @@
> one
> two
> three
> +four
It seems that `git diff` shows the difference between the clean versions
of the files, which is also shown by the GIT_TRACE output when running
it.
> While the diff tool tells me that the repository file is:
> > one
> > two
> > three
This indicates that the smudge filter is being applied, so the
infrastructure is working on your machine.
I think this is a difference between git-diff's internal and external
diff modes which is working correctly, although possibly not desirably
in this case. The internal diff always uses clean files (so it runs the
working tree file through the "clean" filter before applying the diff
algorithm) but the external diff uses the working tree file so it
applies the "smudge" filter to any blobs that it needs to checkout.
Commit 4e218f5 (Smudge the files fed to external diff and textconv,
2009-03-21) was the source of this behaviour.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Using clean/smudge filters with difftool
2015-06-18 16:01 ` John Keeping
@ 2015-06-18 20:00 ` Junio C Hamano
2015-06-18 22:39 ` John Keeping
0 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2015-06-18 20:00 UTC (permalink / raw)
To: John Keeping; +Cc: Florian Aspart, Michael J Gruber, git
John Keeping <john@keeping.me.uk> writes:
> I think this is a difference between git-diff's internal and external
> diff modes which is working correctly, although possibly not desirably
> in this case. The internal diff always uses clean files (so it runs the
> working tree file through the "clean" filter before applying the diff
> algorithm) but the external diff uses the working tree file so it
> applies the "smudge" filter to any blobs that it needs to checkout.
>
> Commit 4e218f5 (Smudge the files fed to external diff and textconv,
> 2009-03-21) was the source of this behaviour.
The fundamental design to use smudged version when interacting with
external programs actually predates that particular commit, I think.
The caller of the function that was updated by that commit, i.e.
prepare_temp_file(), reuses what is checked out to the working tree
when we can (i.e. it hasn't been modified from what we think is
checked out) and when it is beneficial to do so (i.e. on a system
with FAST_WORKING_DIRECTORY defined), which means the temporary file
given by the prepare_temp_file() that is used by the external tools
(both --ext-diff program and textconv filter) are designed to be fed
and work on the smudged version of the file. 4e218f5 did not change
that fundamental design; it just made things more consistent between
the case where we do create a new temporary file out of blob and we
allow an unmodified checked out file to be reused.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Using clean/smudge filters with difftool
2015-06-18 20:00 ` Junio C Hamano
@ 2015-06-18 22:39 ` John Keeping
2015-06-18 22:55 ` Junio C Hamano
0 siblings, 1 reply; 18+ messages in thread
From: John Keeping @ 2015-06-18 22:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Florian Aspart, Michael J Gruber, git
On Thu, Jun 18, 2015 at 01:00:36PM -0700, Junio C Hamano wrote:
> John Keeping <john@keeping.me.uk> writes:
>
> > I think this is a difference between git-diff's internal and external
> > diff modes which is working correctly, although possibly not desirably
> > in this case. The internal diff always uses clean files (so it runs the
> > working tree file through the "clean" filter before applying the diff
> > algorithm) but the external diff uses the working tree file so it
> > applies the "smudge" filter to any blobs that it needs to checkout.
> >
> > Commit 4e218f5 (Smudge the files fed to external diff and textconv,
> > 2009-03-21) was the source of this behaviour.
>
> The fundamental design to use smudged version when interacting with
> external programs actually predates that particular commit, I think.
>
> The caller of the function that was updated by that commit, i.e.
> prepare_temp_file(), reuses what is checked out to the working tree
> when we can (i.e. it hasn't been modified from what we think is
> checked out) and when it is beneficial to do so (i.e. on a system
> with FAST_WORKING_DIRECTORY defined), which means the temporary file
> given by the prepare_temp_file() that is used by the external tools
> (both --ext-diff program and textconv filter) are designed to be fed
> and work on the smudged version of the file. 4e218f5 did not change
> that fundamental design; it just made things more consistent between
> the case where we do create a new temporary file out of blob and we
> allow an unmodified checked out file to be reused.
When I started looking at this, I assumed the problem would be that
git-difftool wasn't smudging the non-working-tree files. But actually
everything is working "correctly", I'm just not sure it's always what
the user wants (at least it isn't what was wanted in this case).
Currently, the behaviour is:
internal diff: compare clean files
external diff: compare smudged files
This makes sense for LF/CRLF conversion, where platform-specific tools
clearly want the platform's line ending but the internal diff machinery
doesn't care.
However, from the filter description in an earlier email, I think
Florian is using a clean filter to remove output from IPython notebook
files (it seems that IPython saves both the input and the output in the
same file [1] and the output is the equivalent of, for example, C object
files). In this case, the filter is one-way and discards information
from the working tree file, producing a smaller and more readable diff
in the process.
I think the summary is that there are some scenarios where the external
diff tool should see the smudged version and others where the clean
version is more appropriate and Git should support both options. It
seems this is a property of the filter, so I wonder if the best solution
is a new "filter.<name>.extdiff = [clean|smudge]" configuration
variable (there's probably a better name for the variable than
"extdiff").
[1] http://pascalbugnion.net/blog/ipython-notebooks-and-git.html
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Using clean/smudge filters with difftool
2015-06-18 22:39 ` John Keeping
@ 2015-06-18 22:55 ` Junio C Hamano
2015-06-19 8:57 ` Michael J Gruber
0 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2015-06-18 22:55 UTC (permalink / raw)
To: John Keeping; +Cc: Florian Aspart, Michael J Gruber, git
John Keeping <john@keeping.me.uk> writes:
> I think the summary is that there are some scenarios where the external
> diff tool should see the smudged version and others where the clean
> version is more appropriate and Git should support both options. It
> seems this is a property of the filter, so I wonder if the best solution
> is a new "filter.<name>.extdiff = [clean|smudge]" configuration
> variable (there's probably a better name for the variable than
> "extdiff").
Not just the external diff, but the textconv filter obeys the same
rule. The setting should be done the same way for both, if we are
going to go in that direction.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Using clean/smudge filters with difftool
2015-06-18 22:55 ` Junio C Hamano
@ 2015-06-19 8:57 ` Michael J Gruber
2015-06-19 9:32 ` John Keeping
2015-06-19 17:03 ` Junio C Hamano
0 siblings, 2 replies; 18+ messages in thread
From: Michael J Gruber @ 2015-06-19 8:57 UTC (permalink / raw)
To: Junio C Hamano, John Keeping; +Cc: Florian Aspart, git
Junio C Hamano venit, vidit, dixit 19.06.2015 00:55:
> John Keeping <john@keeping.me.uk> writes:
>
>> I think the summary is that there are some scenarios where the external
>> diff tool should see the smudged version and others where the clean
>> version is more appropriate and Git should support both options. It
>> seems this is a property of the filter, so I wonder if the best solution
>> is a new "filter.<name>.extdiff = [clean|smudge]" configuration
>> variable (there's probably a better name for the variable than
>> "extdiff").
>
> Not just the external diff, but the textconv filter obeys the same
> rule. The setting should be done the same way for both, if we are
> going to go in that direction.
>
textconv is a "one-way" filter from "blob" to "readable blob". External
diffs may prefer to work on "blob" rather than "readable blob", but the
currect setup does not seem to produce surprises.
clean and smudge are two-way filters: clean from "worktree blob" (aka
file) to "repo blob", smudge the other way round.
Typically, the user perceives these as inverse to each other. But we
only require clean to be a left-inverse of smudge, i.e. "(cat-file then)
smudge then clean" should give the same "repo blob" (as "cat-file").
We don't require that the other way round, i.e. we don't require smudge
to be a left-inverse of clean, and in most setups (like the current one)
it is not: smudge does not recreate what clean has cleaned out. It is a
no-op (the "identity", while clean is a "projection").
Now, since external diff runs on smudged blobs, it appears as if we
mixed cleaned and smudged blobs when feeding external diffs; whereas
really, we mix "worktree blobs" and "smudged repo blobs", which is okay
as per our definition of clean/smudge: the difference is irrelevant by
definition.
I still think that feeding cleaned blobs to external diff would be less
surprising (and should be the default, but maybe can't be changed any
more) and feeding smudged blobs should be the special case requiring a
special config. Because otherwise, the external diff would have to know
which parts of the diff are irrelevant - if it display the complete
("uncleaned") diff, it shows differences ("what will be committed") that
will not end up in the commit (because they will get cleaned out before).
As a guiding principle, a worktree-HEAD diff and an index-HEAD diff
should be previews of the result of "commit -a" resp. "commit", and
therefore should diff cleaned versions. textconv, on the other hand, is
a setting by which you tell git: "Don't show me the 'proper'
diff/commit-preview but a readable version."
Michael
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Using clean/smudge filters with difftool
2015-06-19 8:57 ` Michael J Gruber
@ 2015-06-19 9:32 ` John Keeping
2015-06-19 15:04 ` Florian Aspart
2015-06-19 17:03 ` Junio C Hamano
1 sibling, 1 reply; 18+ messages in thread
From: John Keeping @ 2015-06-19 9:32 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Junio C Hamano, Florian Aspart, git
On Fri, Jun 19, 2015 at 10:57:55AM +0200, Michael J Gruber wrote:
> Junio C Hamano venit, vidit, dixit 19.06.2015 00:55:
> > John Keeping <john@keeping.me.uk> writes:
> >
> >> I think the summary is that there are some scenarios where the external
> >> diff tool should see the smudged version and others where the clean
> >> version is more appropriate and Git should support both options. It
> >> seems this is a property of the filter, so I wonder if the best solution
> >> is a new "filter.<name>.extdiff = [clean|smudge]" configuration
> >> variable (there's probably a better name for the variable than
> >> "extdiff").
> >
> > Not just the external diff, but the textconv filter obeys the same
> > rule. The setting should be done the same way for both, if we are
> > going to go in that direction.
> >
>
> textconv is a "one-way" filter from "blob" to "readable blob". External
> diffs may prefer to work on "blob" rather than "readable blob", but the
> currect setup does not seem to produce surprises.
>
> clean and smudge are two-way filters: clean from "worktree blob" (aka
> file) to "repo blob", smudge the other way round.
>
> Typically, the user perceives these as inverse to each other. But we
> only require clean to be a left-inverse of smudge, i.e. "(cat-file then)
> smudge then clean" should give the same "repo blob" (as "cat-file").
>
> We don't require that the other way round, i.e. we don't require smudge
> to be a left-inverse of clean, and in most setups (like the current one)
> it is not: smudge does not recreate what clean has cleaned out. It is a
> no-op (the "identity", while clean is a "projection").
>
> Now, since external diff runs on smudged blobs, it appears as if we
> mixed cleaned and smudged blobs when feeding external diffs; whereas
> really, we mix "worktree blobs" and "smudged repo blobs", which is okay
> as per our definition of clean/smudge: the difference is irrelevant by
> definition.
I agree with this.
But I was wrong that "should diff clean"/"should diff smudged" is a
property of the filter. I can also imagine a situation where a more
intelligent external diff tool wants to see the smudged version where a
naïve tool would want the clean version.
For example, some of the big file stores (e.g. git-lfs [1]) use
clean/smudge filters and I can imagine a diff utility that avoids
needing to fetch the data for large files and instead shows the diff on
the server when both blobs are available there. In that case we
generally want to use the smudged copy for external diff, so the filter
would use that setting, but the diff utility knows better and would want
to override that.
[1] https://github.com/github/git-lfs
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Using clean/smudge filters with difftool
2015-06-19 9:32 ` John Keeping
@ 2015-06-19 15:04 ` Florian Aspart
0 siblings, 0 replies; 18+ messages in thread
From: Florian Aspart @ 2015-06-19 15:04 UTC (permalink / raw)
To: John Keeping; +Cc: Michael J Gruber, Junio C Hamano, git
2015-06-19 11:32 GMT+02:00 John Keeping <john@keeping.me.uk>:
> On Fri, Jun 19, 2015 at 10:57:55AM +0200, Michael J Gruber wrote:
>> Junio C Hamano venit, vidit, dixit 19.06.2015 00:55:
>> > John Keeping <john@keeping.me.uk> writes:
>> >
>> >> I think the summary is that there are some scenarios where the external
>> >> diff tool should see the smudged version and others where the clean
>> >> version is more appropriate and Git should support both options. It
>> >> seems this is a property of the filter, so I wonder if the best solution
>> >> is a new "filter.<name>.extdiff = [clean|smudge]" configuration
>> >> variable (there's probably a better name for the variable than
>> >> "extdiff").
>> >
>> > Not just the external diff, but the textconv filter obeys the same
>> > rule. The setting should be done the same way for both, if we are
>> > going to go in that direction.
>> >
>>
>> textconv is a "one-way" filter from "blob" to "readable blob". External
>> diffs may prefer to work on "blob" rather than "readable blob", but the
>> currect setup does not seem to produce surprises.
>>
>> clean and smudge are two-way filters: clean from "worktree blob" (aka
>> file) to "repo blob", smudge the other way round.
>>
>> Typically, the user perceives these as inverse to each other. But we
>> only require clean to be a left-inverse of smudge, i.e. "(cat-file then)
>> smudge then clean" should give the same "repo blob" (as "cat-file").
>>
>> We don't require that the other way round, i.e. we don't require smudge
>> to be a left-inverse of clean, and in most setups (like the current one)
>> it is not: smudge does not recreate what clean has cleaned out. It is a
>> no-op (the "identity", while clean is a "projection").
>>
>> Now, since external diff runs on smudged blobs, it appears as if we
>> mixed cleaned and smudged blobs when feeding external diffs; whereas
>> really, we mix "worktree blobs" and "smudged repo blobs", which is okay
>> as per our definition of clean/smudge: the difference is irrelevant by
>> definition.
>
> I agree with this.
>
> But I was wrong that "should diff clean"/"should diff smudged" is a
> property of the filter. I can also imagine a situation where a more
> intelligent external diff tool wants to see the smudged version where a
> naïve tool would want the clean version.
>
> For example, some of the big file stores (e.g. git-lfs [1]) use
> clean/smudge filters and I can imagine a diff utility that avoids
> needing to fetch the data for large files and instead shows the diff on
> the server when both blobs are available there. In that case we
> generally want to use the smudged copy for external diff, so the filter
> would use that setting, but the diff utility knows better and would want
> to override that.
>
> [1] https://github.com/github/git-lfs
I can understand why they are not fed with the clean copy by default.
Since some external diff tool enable modifying the working copy file,
this would correspond to apply the cleaning filter to the working copy
version.
Nevertheless, in my case it would be really helpful if there were an
option to feed the external diff tool with the cleaned version.
Otherwise, I'll probably write a custom script which does this.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Using clean/smudge filters with difftool
2015-06-19 8:57 ` Michael J Gruber
2015-06-19 9:32 ` John Keeping
@ 2015-06-19 17:03 ` Junio C Hamano
2015-06-21 19:29 ` Michael J Gruber
1 sibling, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2015-06-19 17:03 UTC (permalink / raw)
To: Michael J Gruber; +Cc: John Keeping, Florian Aspart, git
Michael J Gruber <git@drmicha.warpmail.net> writes:
> Now, since external diff runs on smudged blobs, it appears as if we
> mixed cleaned and smudged blobs when feeding external diffs; whereas
> really, we mix "worktree blobs" and "smudged repo blobs", which is okay
> as per our definition of clean/smudge: the difference is irrelevant by
> definition.
It does not appear to "mix cleaned and smudged" to me (even though
before Dscho's commit that John pointed out, we did mix by mistake)
to me, but you arrived at the correct conclusion in the rest of your
sentence.
We treat "worktree files" and "smudged repo blobs" as "comparable"
because by definition the latter is what you get if you did a
"checkout" of the blob. Indeed, when we know a worktree file is an
unmodified checkout from a blob and we want to have a read-only
temporary file for a "smudged repo blob", we allow that worktree
file to be used as such.
So in that sense, the commit by Dscho that John pointed out earlier
was not something that changed the semantics; it merely made things
consistent (before that commit, we used to use clean version if we
do not have a usable worktree file).
It is a separate question which of clean or smudged an external diff
tool should be given to work on.
> I still think that feeding cleaned blobs to external diff would be less
> surprising (and should be the default, but maybe can't be changed any
> more) and feeding smudged blobs should be the special case requiring a
> special config.
Go back six years and make a review comment before 4e218f54 (Smudge
the files fed to external diff and textconv, 2009-03-21) was taken
;-). The argument against that commit may have gone like this:
* The current (that is, current as of 4e218f54^) code is
inconsistent, and your patch has a side effect of making it
consistent by always feeding smudged version.
* We however could make it consistent by always feeding clean
version (i.e. disable borrow-from-working-tree codepath when
driving external diff). And that gives us cleaner semantics; the
internal diff and external diff will both work on clean, not
smudged data.
* Of course, going the "clean" way would not help your cause of
allowing external diff to work on smudged version, so you would
need a separate patch on top of that "consistently feed 'clean'
version" fix to optionally allow "consistently feed 'smudge'
version" mode to help msysGit issue 177.
And I would have bought such an argument with 97% chance [*1*].
I do not think 6 years have changed things very much with respect to
the above three-bullet point argument, except that it would be too
late to set the default to 'clean' all of a sudden. So a plausible
way forward would be to
* introduce an option to feed 'clean' versions to external diff
drivers, perhaps with --ext-diff-clean=<driver> command line
option and GIT_EXTERNAL_DIFF_CLEAN environment variable, both of
which take precedence over existing --ext-diff/GIT_EXTERNAL_DIFF
* optionally add a configuration variable diff.feedCleanToExternal
that makes --ext-diff/GIT_EXTERNAL_DIFF behave as if their
'clean' siblings were given. Default it to false.
My gut feeling is that textconv should need a similar treatment for
consistency (after all, it goes through the same prepare_temp_file()
infrastructure).
[Footnote]
*1* The 3% reservation is that I am not entirely convinced that
"both internal and external get to work on the same 'clean'
representation gives us cleaner semantics" is always true.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Using clean/smudge filters with difftool
2015-06-19 17:03 ` Junio C Hamano
@ 2015-06-21 19:29 ` Michael J Gruber
0 siblings, 0 replies; 18+ messages in thread
From: Michael J Gruber @ 2015-06-21 19:29 UTC (permalink / raw)
To: Junio C Hamano; +Cc: John Keeping, Florian Aspart, git
Junio C Hamano venit, vidit, dixit 19.06.2015 19:03:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
>
>> Now, since external diff runs on smudged blobs, it appears as if we
>> mixed cleaned and smudged blobs when feeding external diffs; whereas
>> really, we mix "worktree blobs" and "smudged repo blobs", which is okay
>> as per our definition of clean/smudge: the difference is irrelevant by
>> definition.
>
> It does not appear to "mix cleaned and smudged" to me (even though
> before Dscho's commit that John pointed out, we did mix by mistake)
> to me,
... neither to me. I appears as if you missed the past subjunctive ;)
> but you arrived at the correct conclusion in the rest of your
> sentence.
> We treat "worktree files" and "smudged repo blobs" as "comparable"
> because by definition the latter is what you get if you did a
> "checkout" of the blob. Indeed, when we know a worktree file is an
> unmodified checkout from a blob and we want to have a read-only
> temporary file for a "smudged repo blob", we allow that worktree
> file to be used as such.
>
> So in that sense, the commit by Dscho that John pointed out earlier
> was not something that changed the semantics; it merely made things
> consistent (before that commit, we used to use clean version if we
> do not have a usable worktree file).
>
> It is a separate question which of clean or smudged an external diff
> tool should be given to work on.
>
>> I still think that feeding cleaned blobs to external diff would be less
>> surprising (and should be the default, but maybe can't be changed any
>> more) and feeding smudged blobs should be the special case requiring a
>> special config.
>
> Go back six years and make a review comment before 4e218f54 (Smudge
> the files fed to external diff and textconv, 2009-03-21) was taken
> ;-). The argument against that commit may have gone like this:
>
> * The current (that is, current as of 4e218f54^) code is
> inconsistent, and your patch has a side effect of making it
> consistent by always feeding smudged version.
>
> * We however could make it consistent by always feeding clean
> version (i.e. disable borrow-from-working-tree codepath when
> driving external diff). And that gives us cleaner semantics; the
> internal diff and external diff will both work on clean, not
> smudged data.
>
> * Of course, going the "clean" way would not help your cause of
> allowing external diff to work on smudged version, so you would
> need a separate patch on top of that "consistently feed 'clean'
> version" fix to optionally allow "consistently feed 'smudge'
> version" mode to help msysGit issue 177.
>
> And I would have bought such an argument with 97% chance [*1*].
>
> I do not think 6 years have changed things very much with respect to
> the above three-bullet point argument, except that it would be too
> late to set the default to 'clean' all of a sudden. So a plausible
> way forward would be to
>
> * introduce an option to feed 'clean' versions to external diff
> drivers, perhaps with --ext-diff-clean=<driver> command line
> option and GIT_EXTERNAL_DIFF_CLEAN environment variable, both of
> which take precedence over existing --ext-diff/GIT_EXTERNAL_DIFF
>
> * optionally add a configuration variable diff.feedCleanToExternal
> that makes --ext-diff/GIT_EXTERNAL_DIFF behave as if their
> 'clean' siblings were given. Default it to false.
>
> My gut feeling is that textconv should need a similar treatment for
> consistency (after all, it goes through the same prepare_temp_file()
> infrastructure).
>
>
> [Footnote]
>
> *1* The 3% reservation is that I am not entirely convinced that
> "both internal and external get to work on the same 'clean'
> representation gives us cleaner semantics" is always true.
With consistency stepping back behind compatibility, I don't expect any
defaults to change.
But a knob to change defaults would be nice, yes, and in that case for
external diff as well as textconv. A config variable should suffice
given that we have "git -c" these days.
Michael
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2015-06-21 19:30 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-16 14:11 Using clean/smudge filters with difftool Florian Aspart
2015-06-18 12:31 ` Michael J Gruber
2015-06-18 13:15 ` Florian Aspart
2015-06-18 13:26 ` John Keeping
2015-06-18 13:51 ` Florian Aspart
2015-06-18 14:11 ` John Keeping
2015-06-18 14:17 ` Florian Aspart
2015-06-18 14:28 ` John Keeping
2015-06-18 15:39 ` Florian Aspart
2015-06-18 16:01 ` John Keeping
2015-06-18 20:00 ` Junio C Hamano
2015-06-18 22:39 ` John Keeping
2015-06-18 22:55 ` Junio C Hamano
2015-06-19 8:57 ` Michael J Gruber
2015-06-19 9:32 ` John Keeping
2015-06-19 15:04 ` Florian Aspart
2015-06-19 17:03 ` Junio C Hamano
2015-06-21 19:29 ` Michael J Gruber
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.