Re: [PATCH] Ruby bindings client for merge_peg

Hi Dan, kou,

On 8/17/07, Daniel Rall <dlr@collab.net> wrote:
> On Fri, 17 Aug 2007, Kouhei Sutou wrote:
> ...
> > > > > > def merge_peg(src, rev1, rev2, target_wcpath,
> > > > > > peg_rev=nil, depth=nil, ...)
> > > > > > peg_rev ||= rev2
> > > > > > ...
> > > > > >
> > > > >
> > > > > I thought about that. To be honest I don't understand peg revisions
> > > > > that well, but
> > > > > it seemed like a method called merge_peg might reasonably require a peg revision
> > > > > argument. So if this is a better solution we should just use it instead.....
> > > >
> > > > If there is a reasonable default value, I don't want to
> > > > require the value. In this case, you always use rev2 as
> > > > peg_rev. So it seems that rev2 is a reasonable default
> > > > value. Is it not right?
> > >
> > > Your argument seems correct to me, but I don't know about the
> > > conclusion. I used rev2 because that made merge_peg behave the way
> > > the tests were expecting (looks basically like merge to me).
> > >
> > > I guess the reason I'm hesitating is I'm not sure what the difference
> > > between a merge and a merge_peg is if peg_rev==rev2. The other thing
> > > is the C code makes sure this is not null, if it were reasonable to
> > > have peg_rev assigned to rev2 it could have been done there.
> > >
> > > I've never used merge_peg myself, if you've actually used it and this
> > > was a good default let's just make it the default. On the other hand,
> > > is there someone we can drag into this conversation to be an expert?
> >
> > Me too...
> > I hope someone helps us.
> >
> > For now, could you use rev2 as default? If someone helps us,
> > we may change default value or require a value.
>
> How about following the same algorithm for defaulting the peg revision
> as in subversion/svn/merge-cmd.c?
>
> If a peg revision is unspecified, it's defaulted to HEAD for URLs, and
> WORKING for WC paths.
>
>

That seems like a good idea. This is what it looks like, what do you think?

--
Joe
Index: subversion/bindings/swig/ruby/test/test_client.rb
===================================================================
--- subversion/bindings/swig/ruby/test/test_client.rb   (revision 26199)
+++ subversion/bindings/swig/ruby/test/test_client.rb   (working copy)
@@ -898,7 +898,7 @@
     ctx.merge_peg(branch, rev1, rev2, trunk)
     merge_info = ctx.merge_info(trunk)
     assert_equal(["/branch"], merge_info.keys)
-    assert_equal([[2, 2]], merge_info["/branch"].collect {|range| range.to_a})
+    assert_equal([[1, 2]], merge_info["/branch"].collect {|range| range.to_a})
     rev3 = ctx.commit(@wc_path).revision
     assert_equal(normalize_line_break(src), ctx.cat(trunk_path, rev3))
@@ -911,12 +911,10 @@
     merge_info = ctx.merge_info(trunk, rev4)
     assert_equal(["/branch"], merge_info.keys)
-    assert_equal([[2, 2], [4, 4]],
+    assert_equal([[1, 2], [3, 4]],
                  merge_info["/branch"].collect {|range| range.to_a })
     ctx.propdel("svn:mergeinfo", trunk)
-    merge_info = ctx.merge_info(trunk)
-    assert_equal(["/branch"], merge_info.keys)
-    assert_equal([[2, 2]], merge_info["/branch"].collect {|range| range.to_a})
+    assert_nil ctx.merge_info(trunk)
     ctx.revert(trunk_path)
     File.open(trunk_path, "a") {|f| f.print(src)}
Index: subversion/bindings/swig/ruby/svn/client.rb
===================================================================
--- subversion/bindings/swig/ruby/svn/client.rb (revision 26199)
+++ subversion/bindings/swig/ruby/svn/client.rb (working copy)
@@ -315,10 +315,15 @@
                       dry_run, options, self)
       end
+      def uri? path
+        path=~%r{^[^:]+\://.+}
+      end
+
       def merge_peg(src, rev1, rev2, target_wcpath,
                     peg_rev=nil, depth=nil,
                     ignore_ancestry=false, force=false,
                     dry_run=false, options=nil, record_only=false)
+        peg_rev ||= uri?(src) ? 'HEAD' : 'WORKING'
         Client.merge_peg3(src, rev1, rev2, peg_rev,
                           target_wcpath, depth, ignore_ancestry,
                           force, record_only, dry_run, options, self)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org