On Tue, Mar 12, 2013 at 5:17 PM, John Keeping <john@xxxxxxxxxxxxx> wrote:
> On Tue, Mar 12, 2013 at 04:48:16PM -0600, Matt McClure wrote:
>> On Mar 12, 2013, at 4:16 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>>
>> > Matt McClure <matthewlmcclure@xxxxxxxxx> writes:
>> >
>> > * If you are comparing two trees, and especially if your RHS is not
>> > HEAD, you will send everything to a temporary without
>> > symlinks. Any edit made by the user will be lost.
>>
>> I think you're suggesting to use a symlink any time the content of any
>> given RHS revision is the same as the working tree.
>>
>> I imagine that might confuse me as a user. It would create
>> circumstances where some files are symlinked and others aren't for
>> reasons that won't be straightforward.
>>
>> I imagine solving that case, I might instead implement a copy back to
>> the working tree with conflict detection/resolution. Some earlier
>> iterations of the directory diff feature used copy back without
>> conflict detection and created situations where I clobbered my own
>> changes by finishing a directory diff after making edits concurrently.
>
> The code to copy back working tree files is already there, it just
> triggers using the same logic as the creation of symlinks in the first
> place and doesn't attempt any conflict detection. I suspect that any
> more comprehensive solution will need to restrict the use of "git
> difftool -d" whenever the index contains unmerged entries or when there
> are both staged and unstaged changes, since the merge resolution will
> cause these states to be lost.
>
> The implementation of Junio's suggestion is relatively straightforward
> (this is untested, although t7800 passes, and can probably be improved
> by someone better versed in Perl). Does this work for your original
> scenario?
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?
> -- >8 --
> 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.2.rc2.4.g7799588
>
--
David
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html