threads / patch / 33166

patchdifftool: Make directory diff symlink working tree

Subject: [PATCH] difftool: Make directory diff symlink working tree

## tl;dr

2 messages between Mar 13, 2013 and Mar 13, 2013. Diffs are folded; open one to read it.

replies: 1people: 2as markdown or json

Matt McClure· Mar 13, 2013, 15:21 UTC · lore
On Wed, Mar 13, 2013 at 4:24 AM, David Aguilar <davvid@gmail.com> wrote:
Show 8 quoted lines
> This is a nice straightforward approach.
>
> As Junio mentioned, a good next step would be this patch
> in combination with making the truly temporary files
> created by dir-diff readonly.
>
> Will that need a win32 platform check?
> Does anyone want to take this and whip it into a proper patch?
An attempt:
>From 429ae282ffd7202b6d2fb024a92dea543b8af376 Mon Sep 17 00:00:00 2001
From: Matt McClure <matthewlmcclure@gmail.com>
Date: Wed, 13 Mar 2013 11:14:22 -0400
Subject: [PATCH] difftool: Make directory diff symlink working tree
...primarily so that a user can edit working tree files in his difftool.

difftool -d formerly knew how to symlink to the working tree when the work tree contains uncommitted changes. In practice, prior to this change, it would not symlink to the working tree in case there were no uncommitted changes, even when the user invoked difftool with the form:

    git difftool -d [--options] <commit> [--] [<path>...]
        This form is to view the changes you have in your working tree
        relative to the named <commit>. You can use HEAD to compare it
        with the latest commit, or a branch name to compare with the tip
        of a different branch.

Instead, prior to this change, difftool would use the file's blob SHA1 to find its content in the index rather than use the working tree content. This change teaches `git difftool` to compare the blob SHA1 to the file's working tree blob SHA1 and use the working tree file if the SHA1s are the same.

Author: John Keeping <john@keeping.me.uk>
Conversation:
http://thread.gmane.org/gmane.comp.version-control.git/217979/focus=218014
---
 git-difftool.perl | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)
Show changes to git-difftool.perl +18 −3
diff --git a/git-difftool.perl b/git-difftool.perl
index 0a90de4..5f093ae 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -83,6 +83,21 @@ sub exit_cleanup
 	exit($status | ($status >> 8));
 }

+sub use_wt_file
+{
+	my ($repo, $workdir, $file, $sha1, $symlinks) = @_;
+	my $null_sha1 = '0' x 40;
+
+	if ($sha1 eq $null_sha1) {
+		return 1;
+	} elsif (not $symlinks) {
+		return 0;
+	}
+
+	my $wt_sha1 = $repo->command_oneline('hash-object', "$workdir/$file");
+	return $sha1 eq $wt_sha1;
+}
+
 sub setup_dir_diff
 {
 	my ($repo, $workdir, $symlinks) = @_;
@@ -159,10 +174,10 @@ EOF
 		}

 		if ($rmode ne $null_mode) {
-			if ($rsha1 ne $null_sha1) {
-				$rindex .= "$rmode $rsha1\t$dst_path\0";
-			} else {
+			if (use_wt_file($repo, $workdir, $dst_path, $rsha1, $symlinks)) {
 				push(@working_tree, $dst_path);
+			} else {
+				$rindex .= "$rmode $rsha1\t$dst_path\0";
 			}
 		}
 	}

-- 
1.8.1.5



-- 
Matt McClure
http://www.matthewlmcclure.com
http://www.mapmyfitness.com/profile/matthewlmcclure
John Keeping· Mar 13, 2013, 16:01 UTC · re: Matt McClure · lore

Re: [PATCH] difftool: Make directory diff symlink working tree

On Wed, Mar 13, 2013 at 11:21:40AM -0400, Matt McClure wrote:
Show 18 quoted lines
> On Wed, Mar 13, 2013 at 4:24 AM, David Aguilar <davvid@gmail.com> wrote:
> > This is a nice straightforward approach.
> >
> > As Junio mentioned, a good next step would be this patch
> > in combination with making the truly temporary files
> > created by dir-diff readonly.
> >
> > Will that need a win32 platform check?
> > Does anyone want to take this and whip it into a proper patch?
> 
> An attempt:
> 
> From 429ae282ffd7202b6d2fb024a92dea543b8af376 Mon Sep 17 00:00:00 2001
> From: Matt McClure <matthewlmcclure@gmail.com>
> Date: Wed, 13 Mar 2013 11:14:22 -0400
> Subject: [PATCH] difftool: Make directory diff symlink working tree
> 
> ...primarily so that a user can edit working tree files in his difftool.

Please don't continue the subject like this, look at some recent commits in git.git to see how commits are normally formatted.

The subject here is also inaccurate since difftool already symlinks the working tree in some situations.

Show 16 quoted lines
> difftool -d formerly knew how to symlink to the working tree when the
> work tree contains uncommitted changes. In practice, prior to this
> change, it would not symlink to the working tree in case there were no
> uncommitted changes, even when the user invoked difftool with the form:
> 
>     git difftool -d [--options] <commit> [--] [<path>...]
>         This form is to view the changes you have in your working tree
>         relative to the named <commit>. You can use HEAD to compare it
>         with the latest commit, or a branch name to compare with the tip
>         of a different branch.
> 
> Instead, prior to this change, difftool would use the file's blob SHA1
> to find its content in the index rather than use the working tree
> content. This change teaches `git difftool` to compare the blob SHA1 to
> the file's working tree blob SHA1 and use the working tree file if the
> SHA1s are the same.

You don't need to say things like "formerly..." and "prior to this", write in the imperative describing how the commit changes things. A better commit message might be:

    difftool -d: symlink working tree files matching RHS
    Change the behaviour of git-difftool's directory diff mode so that
    instead of symlinking to working tree files only when they have
    unstaged changes we now symlink to any files where the working tree
    matches the RHS of the diff.
    This helps users who like to edit files in their diff tool and
    expect to have those changes reflected in the working tree.
> Author: John Keeping <john@keeping.me.uk>

The correct way to credit authorship is via a "From: " header at the top of the message. In this particular case, I think this change also requires a documentation update and some test coverage, so it would be appropriate for whoever adds those to take the credit for the commit and add a "Based-on-patch-by: John Keeping <john@keeping.me.uk>" footer.

You should also add your own "Signed-off-by:" line.

If no-one else gets there first I'm hoping to have a bit of time to add the documentation and test case in 4 hours or so.

> Conversation:
> http://thread.gmane.org/gmane.comp.version-control.git/217979/focus=218014

This isn't necessary, just reply to relevant message in that thread and it will show up correctly in people's mail clients.

Show 46 quoted lines
> ---
>  git-difftool.perl | 21 ++++++++++++++++++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/git-difftool.perl b/git-difftool.perl
> index 0a90de4..5f093ae 100755
> --- a/git-difftool.perl
> +++ b/git-difftool.perl
> @@ -83,6 +83,21 @@ sub exit_cleanup
>  	exit($status | ($status >> 8));
>  }
> 
> +sub use_wt_file
> +{
> +	my ($repo, $workdir, $file, $sha1, $symlinks) = @_;
> +	my $null_sha1 = '0' x 40;
> +
> +	if ($sha1 eq $null_sha1) {
> +		return 1;
> +	} elsif (not $symlinks) {
> +		return 0;
> +	}
> +
> +	my $wt_sha1 = $repo->command_oneline('hash-object', "$workdir/$file");
> +	return $sha1 eq $wt_sha1;
> +}
> +
>  sub setup_dir_diff
>  {
>  	my ($repo, $workdir, $symlinks) = @_;
> @@ -159,10 +174,10 @@ EOF
>  		}
> 
>  		if ($rmode ne $null_mode) {
> -			if ($rsha1 ne $null_sha1) {
> -				$rindex .= "$rmode $rsha1\t$dst_path\0";
> -			} else {
> +			if (use_wt_file($repo, $workdir, $dst_path, $rsha1, $symlinks)) {
>  				push(@working_tree, $dst_path);
> +			} else {
> +				$rindex .= "$rmode $rsha1\t$dst_path\0";
>  			}
>  		}
>  	}
> -- 
> 1.8.1.5

← back to recent threads