Re: git diff: add option for omitting the contents of deletes

From: Jeff King <peff@peff.net>
Date: Feb 28, 2011, 22:23 UTC
Message-ID: <20110228222352.GC5854@sigill.intra.peff.net>
In-Reply-To: <7vy650hvwa.fsf@alter.siamese.dyndns.org>

On Mon, Feb 28, 2011 at 10:11:17AM -0800, Junio C Hamano wrote:

Show 10 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > Actually, thinking on this a bit more, I guess "-M" and "-C" are usable
> > without the sha1. In fact, we don't even provide it for a strict 100%
> > rename, and for a rename-with-patch, you can apply the patch, assuming
> > you have the original file in any form.
> 
> Yes, you got it correctly.  A patch is about giving you an ability to
> propagate a change to a similar tree, that does not have to be identical
> to yours (otherwise you can just send tarball of the whole thing ;-)).

Yeah, I guess I am just curious whether "similar enough" is really relevant with file deletion. Obviously if you have the sha1 then everything applies, no problem. But if you don't, how useful is the patch text? The patch application will error out with a conflict. You know in either case that the person wanted the file deleted. The patch text shows you what _their_ version of the file looked like. But that's not likely to be useful; what you really care about is what's in _your_ version of the file, and whether that should be deleted or not.

If you did really care about what was in their file, the giant deletion diff is almost certainly not interesting. What you really care about is what's different in their version versus yours. Which we don't provide a simple way to access. But more likely, you're just going to email them back and say "what is this based on?".

So yeah, the patch gives more information, but I am skeptical that anybody uses it in practice.

Show 6 quoted lines

> And a patch is not purely technical; it has human component in that the
> recipient needs to know enough about the context of the patch to be able
> to judge if the change is applicable to his tree.  By seeing some context
> lines in -M/-C (many of them are not pure renames), the recipient can be
> sure that the patch is being applied to his tree that is "similar enough"
> to what the patch was originally produced against.

But we don't provide context lines for pure renames. Yet the sender might have a different version of the file than the recipient, and the recipient has no idea. We don't even detect that situation, let alone give the user any clue about what might be different.

Show 5 quoted lines

> It would certainly help to have git-apply that understands the rename
> patches to apply such a patch mechanically, but that is not a strict
> requirement; the recipient can move or copy the original file to
> manually create the target and apply the patch by hand to produce the
> desired result, so "do you have git?" is mostly irrelevant.

Isn't that the same for a delete? I can see you wanted the file deleted. It didn't match up with my sha1, but I can go ahead and do the delete manually if I want.

Show 5 quoted lines

> The proposed -D would apply to any tree that happens to have the path
> being mentioned in the patch regardless of how similar the target tree is
> to the original.  Pure renaming -M/-C patch shares the same risk, but
> unlike these modes, -D goes one step too far by making it impossible to
> recover from lossage without having a backup.

I'm not quite sure what lossage you mean. On the recipient's end? They can just "git revert", no? Or do you mean somebody who gets a conflict and manually does the deletion anyway? If they have the patch text, then yes, they have something, but it is _not_ what they deleted. Otherwise they would not have had a conflict.

> But all of the above is only about principles.  As I said, I am not
> strongly opposed to have an output mode that is primarily for reviewing,
> just like we have --color-words, that are not suitable for patch
> application, as long as users understand the implications.

I am not sure I agree about the danger, but certainly I agree that for viewing it is not a problem at all. I have to admit to not caring all _that_ much about the topic. I do find deletion diffs unnecessarily spammy, but they just don't happen enough for me to be really annoyed.

Show 22 quoted lines

> It may make sense to show such a patch still with a bit of context,
> perhaps like this:
> 
>      README |   54 ------------------------------------------------------
>      1 files changed, 0 insertions(+), 54 deletions(-)
> 
>     diff --git -D a/README b/README
>     deleted file mode 100644
>     index 67cfeb2..0000000
>     --- a/README
>     +++ /dev/null
>     @@ -1,54 +0,0 @@
>     -////////////////////////////////////////////////////////////////
>     -
>     -	GIT - the stupid content tracker
>     -...
>     -the discussion following them on the mailing list give a good
>     -reference for project status, development direction and
>     -remaining tasks.
> 
> so that the reader can have some warm and fuzzy feeling that the correct
> file is being deleted, though.

Hmm. I think that is even worse. It's _still_ spammy, and it's broken as a diff (the sha1 in the index line doesn't match the preimage that the diff text shows). Yeah, it gives you warm fuzzy feelings that the file they were thinking about deleting is at least vaguely related to what you wanted to delete. But how often do you get a deletion patch where seeing the first 10 lines is actually useful? That is, what is the case where you see the first 10 lines and immediately say "Wait, this deletion is totally bogus!" and _didn't_ just see that there was a conflicting deletion, read the commit message or email and say "Wait, why are we deleting this file?"

-Peff