[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Show more information in diff labels in mailer.py

From: Mathias Weinert <wein_at_mccw.de>
Date: 2006-09-20 09:58:17 CEST

Peter Samuelson wrote:
>
> I think the parenthesized text is much too verbose:

The initial intention of my patch was exactly to show more verbose
texts and information...

>
> [Mathias Weinert]
> > --- /dev/null 00:00:00 1970 (empty, because file is newly added)
> > +++ dir1/file3 Sun Sep 9 01:46:40 2001 (r1)
>
> For added files, "empty" in place of "rNNN" already implies that the
> file is newly added.
>
> > --- file2 Sun Sep 9 15:40:00 2001 (r5, original)
> > +++ /dev/null 00:00:00 1970 (empty, because file is deleted)
>
> I would abbreviate "(empty, because file is deleted)" as "(deleted)".
>
> > --- dir2/file5 Sun Sep 9 01:46:40 2001 (r1, original)
> > +++ dir2/file5 Sun Sep 9 04:33:20 2001 (r2)
>
> For modified files, "original" is redundant. That is what the --- line
> _always_ means, and anyone who knows unified diff format _knows_ this.

Agreed. But as "original" is already part of the current mailer.py
output I let it stand there.

>
>
> Now for file copies, we have a problem:
>
> > --- file1 Sun Sep 9 01:46:40 2001 (r1, copy source)
> > +++ dir2/file7 Sun Sep 9 07:20:00 2001 (r3, unchanged copied file)
>
> Is this an empty diff (just a diff header and nothing else), or is it
> the full contents of the file? Either way, you cannot expect diff -R
> to do the right thing, unless file1 is labeled as /dev/null.

It is the complete dir2/file7. At least it is since revision 21310.
Since this revision "copied" only reports files that were copied and
not changed and shows the complete file (as if it was added). Before
this revision such files weren't reported at all.

So maybe we should print something like
--- /dev/null 00:00:00 1970 (empty, because file is newly added as a copy of r1, file1 Sun Sep 9 01:46:40 2001)
or
--- /dev/null 00:00:00 1970 (empty, because file is newly added)
because the relevant information can be seen only two lines above this one,
or just
--- /dev/null 00:00:00 1970

>
> > --- file1 Sun Sep 9 01:46:40 2001 (r1, copy source)
> > +++ dir3/file8 Sun Sep 9 10:06:40 2001 (r4)
>
> This one is copy + modify. Is this supposed to show a diff between
> file1 and file8, or the full contents of file8?

It is supposed to show the diff between file1 (r1) and dir3/file8 (r4).
Again have a look at revision 21310.

I have to agree that this is not correct from an "external" point of
view (meaning if you want to apply patches). But always be aware that
this is nothing new! My patch only changes the labels, nothing else (at
least until now).
If you want to use the diffs as input for a patch program we should
provide two diffs. One is the copy as described above and one is the
succeeding modify.
On the other hand as a human reader I will always prefer only one diff
which only contains the changes after the copy action beacuse the
information that this is a new file as a result of a copy I can find
in the enhanced texts in brackets (or in the header of the diff or at
the beginning of the message, as Daniel pointed out).

So what do we have until now?
- Some people like the more verbose texts and some don't.
- Most people only use the diffs for human consumption while several
  use it as input for a patch program.

This again makes me think about an option diff_style or something
similar, althoguh I am not totally convinced that this is necessary.
And BTW it won't help people who cannot set personal diff options as
it is toady for recipients of *the* Subversion repository changes.

Mathias

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Sep 20 09:58:40 2006

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.