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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jeff King <peff@xxxxxxxx> writes:

> 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.

Exactly; in order to intelligently decide if that should be deleted or
not, you need to be able to judge if the deletion makes sense in the
context of _your_ tree.  Hopefully proposed commit log message alone may
clarify if the change applies to your situation, or you need to be able to
see if their version and yours are still serving similar purposes in the
system (e.g. you don't want to lose the definition of a function they
didn't have when they decided to remove the file saying "nobody calls any
function in this library anymore").

> 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.

Correct; the deletion diff has half of the necessary information (the
other half being what you have).

> I'm not quite sure what lossage you mean. On the recipient's end? They
> can just "git revert", no?

No, I was not particularly interested in limiting the discussion to
git-using population.  As I said, just like rename patch can be manually
applied by first moving and then eyeballing, you can see "an instruction
to delete--ok, 'rm'" to apply it and the file is gone (unlike the case
where you manually did "mv").  That is what I meant by "additionally -D
has lossage".

>> 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

That is very much intentional; I don't think we want the format to apply
mechanically.  I was not talking about patch application safety at all in
this thread; I already dismissed "-D" as too unreliable as a patch format
to be used for conveying a change for mechanical consumption, didn't I?
When you want to do so, you just generate the output without -D.

The suggestion to give some context is only for people who want to eyeball
their own patches, helping them to make sure that they are removing what
they wanted to remove.  If they can tell from the filename alone, that is
just as fine.  I am not interested that deeply here; the only thing I care
about is to make sure we don't have to keep carrying crappy options as if
they are sane.
--
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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]