Re: [PATCH v2] difftool --dir-diff: always use identical working tree file
- From
John Keeping <john@keeping.me.uk>
- Date
- May 28, 2013, 18:15 UTC
- Message-ID
- <20130528181525.GA17475@serenity.lan>
- In-Reply-To
- <7vbo7v0yju.fsf@alter.siamese.dyndns.org>
On Tue, May 28, 2013 at 11:06:13AM -0700, Junio C Hamano wrote:
Show 17 quoted lines
> Kenichi Saita <nitoyon@gmail.com> writes: > > > When deciding whether or not we should link a working tree file into > > the temporary right-hand directory for a directory diff, we > > currently behave differently in the --symlink and --no-symlink > > cases. If using symlinks any identical files are linked across but > > with --no-symlink only files that contain unstaged changes are > > copied back into the working tree. > > I may have missed an earlier discussion, but I do not follow the > last sentence. The former part (i.e. symlinks) talks about what is > done to populate the temporary tree (i.e. everything is linked), but > the latter part (i.e. not symlinks) only talks about what is copied > back, i.e. it is not a contrast between the behaviour of symlink vs > no-symlink case wrt how the temporary tree is populated. > > Confused...
Yeah, the commit message is still quite focused on the end effect of copying files back. But that's not what's being changed here.
In my suggested commit message I tried to make it clear that we're changing when we decide to copy a file across to the temporary tree. This has the beneficial (side-)effect of changing the set of files we consider for copying back into the working tree after the diff tool has been run.
Show 76 quoted lines
> > Change this so that identical files are copied back as well. This
> > is beneficial because it widens the set of circumstances in which we
> > copy changes made by the user back into the working tree.
>
> Ah, OK, you meant that the set of files we keep in @working_tree
> array for later copying back are different between the two.
>
> > Signed-off-by: Kenichi Saita <nitoyon@gmail.com>
> > ---
> > git-difftool.perl | 9 ++-------
> > t/t7800-difftool.sh | 19 +++++++++++++++++++
> > 2 files changed, 21 insertions(+), 7 deletions(-)
> >
> > diff --git a/git-difftool.perl b/git-difftool.perl
> > index 8a75205..e57d3d1 100755
> > --- a/git-difftool.perl
> > +++ b/git-difftool.perl
> > @@ -85,13 +85,9 @@ sub exit_cleanup
> >
> > sub use_wt_file
> > {
> > - my ($repo, $workdir, $file, $sha1, $symlinks) = @_;
> > + my ($repo, $workdir, $file, $sha1) = @_;
> > my $null_sha1 = '0' x 40;
> >
> > - if ($sha1 ne $null_sha1 and not $symlinks) {
> > - return 0;
> > - }
> > -
> > if (! -e "$workdir/$file") {
> > # If the file doesn't exist in the working tree, we cannot
> > # use it.
> > @@ -213,8 +209,7 @@ EOF
> >
> > if ($rmode ne $null_mode) {
> > my ($use, $wt_sha1) = use_wt_file($repo, $workdir,
> > - $dst_path, $rsha1,
> > - $symlinks);
> > + $dst_path, $rsha1);
> > if ($use) {
> > push @working_tree, $dst_path;
> > $wtindex .= "$rmode $wt_sha1\t$dst_path\0";
> > diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
> > index d46f041..2418528 100755
> > --- a/t/t7800-difftool.sh
> > +++ b/t/t7800-difftool.sh
> > @@ -385,6 +385,25 @@ test_expect_success PERL,SYMLINKS 'difftool --dir-diff --symlink without unstage
> > test_cmp actual expect
> > '
> >
> > +write_script modify-right-file <<\EOF
> > +echo "new content" >"$2/file"
> > +EOF
> > +
> > +run_dir_diff_test 'difftool --dir-diff syncs worktree with unstaged change' '
> > + test_when_finished git reset --hard &&
> > + echo "orig content" >file &&
> > + git difftool -d $symlinks --extcmd "$(pwd)/modify-right-file" branch &&
> > + echo "new content" >expect &&
> > + test_cmp expect file
> > +'
> > +
> > +run_dir_diff_test 'difftool --dir-diff syncs worktree without unstaged change' '
> > + test_when_finished git reset --hard &&
> > + git difftool -d $symlinks --extcmd "$(pwd)/modify-right-file" branch &&
> > + echo "new content" >expect &&
> > + test_cmp expect file
> > +'
> > +
> > write_script modify-file <<\EOF
> > echo "new content" >file
> > EOF
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html