You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Sander Striker <st...@apache.org> on 2002/04/27 13:14:17 UTC
Unified context diff...(2) WAS: RE: svn commit: rev 1802 - trunk/subversion/libsvn_delta
Hi,
> Author: striker
> Date: 2002-04-27 12:49 GMT
> New Revision: 1802
>
> Modified:
> trunk/subversion/libsvn_delta/diff_file.c
> Log:
> * subversion/libsvn_delta/diff_file.c
>
> (svn_diff__file_output_unified_line): Add a comment as to when
> '\ No newline at end of file' is added.
>
> (svn_diff__file_output_unified_flush_hunk): Add and update comments.
> Break long lines. Use apr_file_write instead of apr_file_printf
> for output of the hunk content. Also add error checking to that
> operation.
>
> (svn_diff__file_output_unified_default_hdr): Use a tab rather than
> three spaces to seperate the path from the last modified time.
This last change gave me:
$ ./diff-striker original modified >output_striker
$ diff -u original modified >output_diff
$ diff output_striker output_diff
[nothing was displayed]
FYI, the diff-striker executable was built from the source I posted in
<JL...@apache.org> "Unified context diff...".
I think I have nailed all edge cases now, but it would be nice to be
sure. Please people, test the crap out of this! :)
Thanks,
Sander
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Unified context diff...(2) WAS: RE: svn commit: rev 1802 - trunk/subversion/libsvn_delta
Posted by Philip Martin <ph...@codematters.co.uk>.
"Kirby C. Bohling" <kb...@birddog.com> writes:
> > svn generates too much leading context.
>
> >
>
>
> Uhhh, that is a feature, not a bug IMHO. One of the things I have to
> work around in CVS, is I can't tell it when merging I want you to use
> 8-10 lines of context.
A controllable amout of context is a feature. Too much context for the
first hunk is a bug.
--
Philip
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Unified context diff...(2) WAS: RE: svn commit: rev 1802 - trunk/subversion/libsvn_delta
Posted by Peter Davis <pe...@pdavis.cx>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On Monday 29 April 2002 11:24, Kirby C. Bohling wrote:
> So maybe I should state that more directly. Would it be possible to have
> anything that does a diff be able to take parameters to pass to the diff
> utility/library? I would very much like to have finer control over the
> diff parameters on certain occasions rather then doing svn/cvs diff
> followed by a manual patch. Especially because svn does a much better
> job of merge history then CVS ever did.
It sounds like this only really is an issue with one (or a few) files, but it
is a valid request (now you got me worried about what will happen with my
code :->). Suggestion: would it be possible to configure some of the params
as the file's properties? For example: "svn:diff-context=8" for the
offending file, with default of "3". When diffing multiple files or an
entire tree, svn would use the greatest value of svn:diff-context out of
those files.
This would make it so nobody has to remember the params every time they diff,
and I think it would just be pretty darn cool. Maybe this is not feasible,
just thought I'd through it out there for discussion.
- --
Peter Davis
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.0.6 (GNU/Linux)
Comment: For info see http://www.gnupg.org
iD8DBQE8zZnwNSZCJx7tYycRApUWAKCEh/QnH19cIpGPrRGr5y6hru0rxACdEepk
/P6zXaGZ3pU4719y+/7c66w=
=blu7
-----END PGP SIGNATURE-----
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Unified context diff...(2) WAS: RE: svn commit: rev 1802 - trunk/subversion/libsvn_delta
Posted by Karl Fogel <kf...@newton.ch.collab.net>.
"Kirby C. Bohling" <kb...@birddog.com> writes:
> Essentially anytime diff/diff3 is called, I like a chance to hint at
> how to do the work. If I had to remember to do it each time, I can do
> that too. 98% of the time the defaults are good enough. When I
> realize they aren't, I'll start over and override the defaults. It's
> the 2% case that is a real bear, and normally those are the cases will
> introduce really subtle bugs in my code base because I missed
> something because the conflict markers made it very hard to see the
> changes. It's why I used cvs diff and patch by hand. IMNSHO, if I
> can get diff and patch to do it handily an SCM should allow me to do
> inside of the SCM.
>
> Granted, this is the part where I should put my code written
> skills where my mouth is and contribute the patch.
:-) See http://subversion.tigris.org/issues/show_bug.cgi?id=647.
We don't invoke patch at all, and we only invoke `diff' for "svn
diff", for which one can already pass arbitrary options.
For your application, diff3 is the issue.
If you can find the parameters you want for diff3, and work out if/how
they will/won't interfere with the parameters we _must_ pass to diff3,
then we can fix this, yeah...
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Unified context diff...(2) WAS: RE: svn commit: rev 1802 - trunk/subversion/libsvn_delta
Posted by "Kirby C. Bohling" <kb...@birddog.com>.
Karl,
Karl Fogel wrote:
> "Kirby C. Bohling" <kb...@birddog.com> writes:
>
I think you missed by big gripe. Sometimes internally CVS and
Subversion use diff/diff3/patch and I can't control the parameters to
it. So for update and update -j in CVS I didn't get to tell CVS what
parameters to use for the diff and patch. I could have fixed my
problems thru CVS if I had control, but I didn't. The cvs diff command
allowed exactly what I wanted to do with update and update -j.
>>Uhhh, that is a feature, not a bug IMHO. One of the things I have to
>>work around in CVS, is I can't tell it when merging I want you to use
>>8-10 lines of context. (Okay so now I will probably have 10 people
>>tell me how I could have done it, which I would very very much
>>appreciate if they did).
>>
>
> That's not a `diff' problem, it's `diff3' or `patch' problem.
>
> And in fact, regular diff allows you to specify how many lines of
> context it generates. The issue is getting patch to *use* that much
> context (or, in our case, the issue is that Sander's replacement for
> `diff3' is more important than the replacement for `diff').
>
Okay, I forgot, with diff3 you get both files, so the context handling
should be better in svn then CVS. I haven't ever used diff3 by hand so
it might solve all my issues.
It is a diff problem, if the diff didn't give you the context, patch
couldn't use it. I'll go build test cases and try it out with diff3.
>
>> So maybe I should state that more directly. Would it be
>>possible to have anything that does a diff be able to take parameters
>>to pass to the diff utility/library? I would very much like to have
>>finer control over the diff parameters on certain occasions rather
>>
>
> You have that right now:
>
> svn diff -x "options to diff go here" ...
>
I know I get that, but for CVS, I didn't get that on a
cvs co module
cd module
cvs update -j branchHead
Where exactly did I get to pass the diff/patch options for pattern matching?
Now I could do this:
cvs diff -r lastMerge -r branchHead "diffOptions" module > branch.patch
cvs co module
cd modules
patch "patchOptions" < branch.patch
Now this looks like local edits to the trunk (which in CVS on a update -j
it always did). If the svn history takes off, that will make this even
harder to do and have svn keep accurate merge history. All because I
can't tweak out the parameters the way I want inside of the tool.
I had no control over the parameters used for the merge ( I always
thought CVS used straight diff/patch possibly some rcs version of those,
I could be completely wrong. Everything I know about CVS I read out of
your book, so you're probably more knowledgable then I am :-).
I have a ton of very similar code for 4 lines in a row, I would like the
ability to tweak out the diff/diff3/patch options to say 4 lines of
match isn't enough try 6-10, and hopefully say oh yeah skip whitespace
in there too. GNU diff's defaults aren't always ideal for all the code.
I haven't looked at Sander's diff code so the options might not be there.
So for example if this worked, it be great:
svn update --diff3Options="use lots of context"
svn switch --diff3Options="use lots of context"
Or be able to say, look for at least 1-8K for matches, if that takes 10
minutes, I'll wait 10 minutes I just want it done correctly by the
computer that is good at the repetative work not by me the lazy bum.
Essentially anytime diff/diff3 is called, I like a chance to hint at how
to do the work. If I had to remember to do it each time, I can do that
too. 98% of the time the defaults are good enough. When I realize they
aren't, I'll start over and override the defaults. It's the 2% case
that is a real bear, and normally those are the cases will introduce
really subtle bugs in my code base because I missed something because
the conflict markers made it very hard to see the changes. It's why I
used cvs diff and patch by hand. IMNSHO, if I can get diff and patch to
do it handily an SCM should allow me to do inside of the SCM.
Granted, this is the part where I should put my code written skills where
my mouth is and contribute the patch.
> Note: I'm not saying Sander's diff work isn't a good idea, just that
> it's not giving us any functionality we don't already have. And until
> it supports all the fancy diff options people use, like -b and -B and
> -F, we'll unfortunately lose functionality by switching to it. And
> replacing diff only affects with "svn diff".
Right new code, might be feature lite. No knobs just yet.
<snip>
>
>>then doing svn/cvs diff followed by a manual patch. Especially
>>because svn does a much better job of merge history then CVS ever did.
>>
>
> s/does/will do/ :-)
>
*grin*
I forget that isn't here just yet.
> -K
Thanks,
Kirby
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Unified context diff...(2) WAS: RE: svn commit: rev 1802 - trunk/subversion/libsvn_delta
Posted by Karl Fogel <kf...@newton.ch.collab.net>.
"Kirby C. Bohling" <kb...@birddog.com> writes:
> Uhhh, that is a feature, not a bug IMHO. One of the things I have to
> work around in CVS, is I can't tell it when merging I want you to use
> 8-10 lines of context. (Okay so now I will probably have 10 people
> tell me how I could have done it, which I would very very much
> appreciate if they did).
That's not a `diff' problem, it's `diff3' or `patch' problem.
And in fact, regular diff allows you to specify how many lines of
context it generates. The issue is getting patch to *use* that much
context (or, in our case, the issue is that Sander's replacement for
`diff3' is more important than the replacement for `diff').
> So maybe I should state that more directly. Would it be
> possible to have anything that does a diff be able to take parameters
> to pass to the diff utility/library? I would very much like to have
> finer control over the diff parameters on certain occasions rather
You have that right now:
svn diff -x "options to diff go here" ...
Note: I'm not saying Sander's diff work isn't a good idea, just that
it's not giving us any functionality we don't already have. And until
it supports all the fancy diff options people use, like -b and -B and
-F, we'll unfortunately lose functionality by switching to it. And
replacing diff only affects with "svn diff".
For SVN, his replacement of "diff3" is what we should really be
testing, because:
a) It's used more often (every update!)
b) A bug would usually result in corrupted local files, rather than
just wrong user-viewed output (they're both bad, but the first
is worse)
b) We know exactly which options we use; whereas with "svn diff"
the user may want any of the zillion options supported by GNU
diff, leaving us with the burden of reimplementing all of those
if we internalize `diff' as well as `diff3'.
If we're going to test Sander's new code, test the diff3 stuff.
That's what we really need. The plain diff replacement has a lot
farther to go before it's useful.
> then doing svn/cvs diff followed by a manual patch. Especially
> because svn does a much better job of merge history then CVS ever did.
s/does/will do/ :-)
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Unified context diff...(2) WAS: RE: svn commit: rev 1802 - trunk/subversion/libsvn_delta
Posted by "Kirby C. Bohling" <kb...@birddog.com>.
Philip Martin wrote:
> "Sander Striker" <st...@apache.org> writes:
>
>
>>I think I have nailed all edge cases now, but it would be nice to be
>>sure. Please people, test the crap out of this! :)
>>
>
> Well, it would be easier with a test suite :-) No point testing the
> stuff you have already tested. However
>
> $ cat zz1
> 999
> 999
> 999
> 999
> 999
> 0
> 1
> 2
> 2
> 1
> 999
> 999
> 999
> $ cat zz2
> 999
> 999
> 999
> 999
> 999
> 0
> 4
> 4
> 0
> 4
> 999
> 999
> 999
> $ ./diff-svn zz1 zz2
> --- zz1 Mon Apr 29 18:35:22 2002
> +++ zz2 Mon Apr 29 18:35:28 2002
> @@ -1,13 +1,13 @@
> 999
> 999
> 999
> 999
> 999
> 0
> -1
> -2
> -2
> -1
> +4
> +4
> +0
> +4
> 999
> 999
> 999
> $ diff -u zz1 zz2
> --- zz1 Mon Apr 29 18:35:22 2002
> +++ zz2 Mon Apr 29 18:35:28 2002
> @@ -4,10 +4,10 @@
> 999
> 999
> 0
> -1
> -2
> -2
> -1
> +4
> +4
> +0
> +4
> 999
> 999
> 999
>
> svn generates too much leading context.
>
>
Uhhh, that is a feature, not a bug IMHO. One of the things I have to
work around in CVS, is I can't tell it when merging I want you to use
8-10 lines of context. (Okay so now I will probably have 10 people tell
me how I could have done it, which I would very very much appreciate if
they did).
Essentially, I have a ton of database code that all looks like this:
void functionName( arg, arg, arg )
{
Result result;
Query query;
Connection connection
Arguments arg;
/* Now put code here.*/
}
void functionName2( arg, arg, arg )
{
Result result;
Query query;
Connection connection;
Arguments arg;
/* Put code here*/
}
I have one file with probably 50 of these functions that have the same
four lines of declarations in the same order named exactly the same way.
Well because the context is so very small I will get conflict from
every function in the file after a small change. At 3-4 lines, I always
match on those for variable declarations. So if I add a function in the
middle of the list (near the related functions). Essentially if you add
a functionName3 between the two, it woud say you removed functionName2
and replaced it with functionName3 then add functionName2 down below.
If you made a change in functionName2 it was much, much harder to spot
because the context diff matched on the 3-4 lines of context for that
file on the 4 idential variable declarations. So the new version of
functionName2 and the old version of functionName2 never got compared.
The old version of functionName2 go compared with the new version of
function3. Not very useful, as they are completely unrelated. So I
would manually examine the diff looking for changes in functionName2.
When doing repeated merging to and from a branch I had outstanding for 8
months it was very difficult. Ever single line of the file had to be
examined during merge conflicts. Which was really annoying because the
this was the only file I was modifying significantly in the branch. In
the end, I found it easier to take the diff's using cvs diff where I
could pass it all the goofy parameters I wanted for context and then use
patch to apply it with fewer, smaller and simpler conflicts. I was
perfactly happy to wait for it to use more context, and was willing to
deal with lots of small changes close together where there wouldn't be
5-8 lines of context I used. Generally those didn't cross function
boundaries which made life much easier.
So maybe I should state that more directly. Would it be possible to have
anything that does a diff be able to take parameters to pass to the diff
utility/library? I would very much like to have finer control over the
diff parameters on certain occasions rather then doing svn/cvs diff
followed by a manual patch. Especially because svn does a much better
job of merge history then CVS ever did.
Thanks,
Kirby
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Unified context diff...(2) WAS: RE: svn commit: rev 1802 - trunk/subversion/libsvn_delta
Posted by Philip Martin <ph...@codematters.co.uk>.
"Sander Striker" <st...@apache.org> writes:
> I think I have nailed all edge cases now, but it would be nice to be
> sure. Please people, test the crap out of this! :)
Well, it would be easier with a test suite :-) No point testing the
stuff you have already tested. However
$ cat zz1
999
999
999
999
999
0
1
2
2
1
999
999
999
$ cat zz2
999
999
999
999
999
0
4
4
0
4
999
999
999
$ ./diff-svn zz1 zz2
--- zz1 Mon Apr 29 18:35:22 2002
+++ zz2 Mon Apr 29 18:35:28 2002
@@ -1,13 +1,13 @@
999
999
999
999
999
0
-1
-2
-2
-1
+4
+4
+0
+4
999
999
999
$ diff -u zz1 zz2
--- zz1 Mon Apr 29 18:35:22 2002
+++ zz2 Mon Apr 29 18:35:28 2002
@@ -4,10 +4,10 @@
999
999
0
-1
-2
-2
-1
+4
+4
+0
+4
999
999
999
svn generates too much leading context.
--
Philip
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org