You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Shlomi Fish <sh...@vipe.stud.technion.ac.il> on 2003/07/05 10:46:04 UTC

Issue 1241 - svnlook diff not printing property diffs

Trying to find a bite-sized issue for me to fix, I ran across Issue 1241:

http://subversion.tigris.org/issues/show_bug.cgi?id=1241

In it the problem is that svnlook diff does not print property diffs.
However, it seems that John Szakmeister supplied a patch to it:

http://www.contactor.se/~dast/svn/archive-2003-05/0757.shtml

My question is: was the patch applied yet? If not, why not? If so, why
wasn't the issue closed?

Regards,

	Shlomi Fish


----------------------------------------------------------------------
Shlomi Fish        shlomif@vipe.technion.ac.il
Home Page:         http://t2.technion.ac.il/~shlomif/

An apple a day will keep a doctor away. Two apples a day will keep two
doctors away.

	Falk Fish

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Issue 1241 - svnlook diff not printing property diffs

Posted by John Szakmeister <jo...@szakmeister.net>.
Please, shoot any feedback my way.  If you have a copy of the re-worked patch 
that would be a nice thing to look at too.

-John

On Tuesday 08 July 2003 02:07 am, cmpilato@collab.net wrote:
> John Szakmeister <jo...@szakmeister.net> writes:
> > Actually, I've been dying for some feedback about the patch.
>
> ...And I've been meaning to review it.  I think I'll do so now.  Ah,
> yes -- this is the one where you copied a bunch of functions from one
> module to another.  I remember being bummed that we didn't just find a
> nice place to expose the original functions once, and use them in both
> places.  Oh, and I see you overlooked (or ignored) the fact that
> svnlook/main.c uses the fn-space-paren style.
>
> But tonight, I'm feeling good, so I'll just rework your patch the way
> I would have liked to see it.  You get a free one, on me.  :-)
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Issue 1241 - svnlook diff not printing property diffs

Posted by John Szakmeister <jo...@szakmeister.net>.
On Tuesday 08 July 2003 10:42 am, cmpilato@collab.net wrote:
> John Szakmeister <jo...@szakmeister.net> writes:
> > I wrote a regression test here after reading Philip's email.  Would you
> > like it?  All I did was compare svnlook's diff to svn's diff to make sure
> > they both did the same thing.
>
> Sure, send it on.  In the meantime, I'll commit up the feature.

Here it is.  It's not much of a test since if both 'svn diff' and 'svnlook 
diff' fails to produce the proper output, the test will pass... but it's 
better than nothing! :-)

-John

Re: Issue 1241 - svnlook diff not printing property diffs

Posted by cm...@collab.net.
John Szakmeister <jo...@szakmeister.net> writes:

> I wrote a regression test here after reading Philip's email.  Would you like 
> it?  All I did was compare svnlook's diff to svn's diff to make sure they 
> both did the same thing.

Sure, send it on.  In the meantime, I'll commit up the feature.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Issue 1241 - svnlook diff not printing property diffs

Posted by John Szakmeister <jo...@szakmeister.net>.
I wrote a regression test here after reading Philip's email.  Would you like 
it?  All I did was compare svnlook's diff to svn's diff to make sure they 
both did the same thing.

-John

On Tuesday 08 July 2003 04:20 am, cmpilato@collab.net wrote:
> cmpilato@collab.net writes:
> > John Szakmeister <jo...@szakmeister.net> writes:
> > > Actually, I've been dying for some feedback about the patch.
> >
> > ...And I've been meaning to review it.  I think I'll do so now.  Ah,
> > yes -- this is the one where you copied a bunch of functions from one
> > module to another.  I remember being bummed that we didn't just find a
> > nice place to expose the original functions once, and use them in both
> > places.  Oh, and I see you overlooked (or ignored) the fact that
> > svnlook/main.c uses the fn-space-paren style.
> >
> > But tonight, I'm feeling good, so I'll just rework your patch the way
> > I would have liked to see it.  You get a free one, on me.  :-)
>
> By the way, look for this commit tomorrow.  Running tests now, but too
> sleepy to wait up for them.  *yawn*
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Issue 1241 - svnlook diff not printing property diffs

Posted by cm...@collab.net.
cmpilato@collab.net writes:

> John Szakmeister <jo...@szakmeister.net> writes:
> 
> > Actually, I've been dying for some feedback about the patch.  
> 
> ...And I've been meaning to review it.  I think I'll do so now.  Ah,
> yes -- this is the one where you copied a bunch of functions from one
> module to another.  I remember being bummed that we didn't just find a
> nice place to expose the original functions once, and use them in both
> places.  Oh, and I see you overlooked (or ignored) the fact that
> svnlook/main.c uses the fn-space-paren style.
> 
> But tonight, I'm feeling good, so I'll just rework your patch the way
> I would have liked to see it.  You get a free one, on me.  :-)

By the way, look for this commit tomorrow.  Running tests now, but too
sleepy to wait up for them.  *yawn*

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Issue 1241 - svnlook diff not printing property diffs

Posted by cm...@collab.net.
John Szakmeister <jo...@szakmeister.net> writes:

> Actually, I've been dying for some feedback about the patch.  

...And I've been meaning to review it.  I think I'll do so now.  Ah,
yes -- this is the one where you copied a bunch of functions from one
module to another.  I remember being bummed that we didn't just find a
nice place to expose the original functions once, and use them in both
places.  Oh, and I see you overlooked (or ignored) the fact that
svnlook/main.c uses the fn-space-paren style.

But tonight, I'm feeling good, so I'll just rework your patch the way
I would have liked to see it.  You get a free one, on me.  :-)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Issue 1241 - svnlook diff not printing property diffs

Posted by John Szakmeister <jo...@szakmeister.net>.
Actually, I've been dying for some feedback about the patch.  I have no 
problem making a regression test if you don't mind helping me out a bit by 
answering a couple of questions.

First, what's the best way to check the output of the svnlook diff command.  
Is it enough that I check the output for the word 'Property' and make sure 
that a property and value of my choice is in the output?  Or would it be 
better to compare it svn diff?  I've already hacked my local copy of 
svnlook_tests.py, I just need a little guidance on the preffered method of 
validating the output.

Thanks!

-John

On Monday 07 July 2003 01:12 pm, Philip Martin wrote:
> Shlomi Fish <sh...@vipe.stud.technion.ac.il> writes:
> > Trying to find a bite-sized issue for me to fix, I ran across Issue 1241:
> >
> > http://subversion.tigris.org/issues/show_bug.cgi?id=1241
> >
> > In it the problem is that svnlook diff does not print property diffs.
> > However, it seems that John Szakmeister supplied a patch to it:
> >
> > http://www.contactor.se/~dast/svn/archive-2003-05/0757.shtml
> >
> > My question is: was the patch applied yet? If not, why not?
>
> One problem with the patch is that the link refers to the contactor
> archive, and the patch there is mangled.  To apply the patch it would
> be necessary to track down a raw patch (from tigris, marc or gmane
> say), or to fix the patch by hand.
>
> In addition there is no regression test which means that whoever
> applies the patch is going to have to work out how to manually test it
> at the very least.  Assuming it does work, without a regression test
> there is no guarantee that it will continue to work in the future.
>
> You say you are looking for a bite-sized issue to fix, well there is
> still work to be done on this one :)


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Issue 1241 - svnlook diff not printing property diffs

Posted by Philip Martin <ph...@codematters.co.uk>.
Shlomi Fish <sh...@vipe.stud.technion.ac.il> writes:

> Trying to find a bite-sized issue for me to fix, I ran across Issue 1241:
>
> http://subversion.tigris.org/issues/show_bug.cgi?id=1241
>
> In it the problem is that svnlook diff does not print property diffs.
> However, it seems that John Szakmeister supplied a patch to it:
>
> http://www.contactor.se/~dast/svn/archive-2003-05/0757.shtml
>
> My question is: was the patch applied yet? If not, why not?

One problem with the patch is that the link refers to the contactor
archive, and the patch there is mangled.  To apply the patch it would
be necessary to track down a raw patch (from tigris, marc or gmane
say), or to fix the patch by hand.

In addition there is no regression test which means that whoever
applies the patch is going to have to work out how to manually test it
at the very least.  Assuming it does work, without a regression test
there is no guarantee that it will continue to work in the future.

You say you are looking for a bite-sized issue to fix, well there is
still work to be done on this one :)

-- 
Philip Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Issue 1241 - svnlook diff not printing property diffs

Posted by kf...@collab.net.
Shlomi Fish <sh...@vipe.stud.technion.ac.il> writes:
> http://www.contactor.se/~dast/svn/archive-2003-05/0757.shtml
> 
> My question is: was the patch applied yet? If not, why not? If so, why
> wasn't the issue closed?

Just because no committer has gotten around to it yet.  When the issue
gets scheduled for a numbered milestone (or when someone gets a spare
half-hour or hour), then it will be applied.

-K

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org