You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Joe Swatosh <jo...@gmail.com> on 2007/05/14 22:52:21 UTC
[patch] ruby bindings tracking the recurse=>depth changes in svn_client_proplist3
Do we want to try to keep up with the recurse to depth changes?
--
Joe
[[[
Follow on to r25007 which added depth support to svn_client_proplist3.
* subversion/bindings/swig/ruby/svn/client.rb
(Svn::Client::Context#proplist) changed argument name and default to
reflect the use of depth instead of recurse in the core API.
]]]
Index: subversion/bindings/swig/ruby/svn/client.rb
===================================================================
--- subversion/bindings/swig/ruby/svn/client.rb (revision 25014)
+++ subversion/bindings/swig/ruby/svn/client.rb (working copy)
@@ -215,7 +215,7 @@
# Returns list of properties attached to +target+ as an Array of
# Svn::Client::PropListItem.
# Paths and URIs are available as +target+.
- def proplist(target, rev=nil, peg_rev=nil, recurse=true, &block)
+ def proplist(target, rev=nil, peg_rev=nil,
depth=Core::DEPTH_INFINITY, &block)
rev ||= "HEAD"
peg_rev ||= rev
items = []
@@ -223,7 +223,7 @@
items << PropListItem.new(path, prop_hash)
block.call(path, prop_hash) if block
end
- Client.proplist3(target, rev, peg_rev, recurse, receiver, self)
+ Client.proplist3(target, rev, peg_rev, depth, receiver, self)
items
end
alias prop_list proplist
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [patch] ruby bindings tracking the recurse=>depth changes in svn_client_proplist3
Posted by Kouhei Sutou <ko...@cozmixng.org>.
Hi,
2007/5/15, Daniel Rall <dl...@collab.net>:
> > >FWIW, I like the use of an explicit value, as it makes the code more
> > >easy to understand.
> >
> > We use depth=nil as default value in other methods.
> > depth=nil means "we use default depth value" and the default
> > depth value is used all depth parameter. I think using depth=nil
> > for all methods is more understandable rather than using
> > depth=Core::DEPTH_INFINITY only for Svn::Client#proplist.
>
> Sure, I understand that nil equates to "use the default" -- that's a
> common pattern. I don't see how nil can be claimed as more
> comprehensible than an explicit value, however, when the latter is
> self-documenting, and the former isn't documented at all in the
> "end-user" API.
Yes. You're right. I'll do what you say if it doesn't affect API usage.
If there is a method like the following:
def x(a, b, c=1, d=2, e=3)
...
end
And I want to specify a, b, and d and use default value for c and e.
Then I'll want to write the following:
x(1, 2, nil, 3)
But I can't do the above because I can't specify d without dropping
c argument. So I can't use default value for c.
But if x is defined as the following:
def x(a, b, c=nil, d=nil, e=nil)
c ||= 1
d ||= 2
e ||= 3
...
end
I can specify d and use default value for c like the following:
x(1, 2, nil, 3)
In this case, it's OK for me to change like the following:
def proplist(target, rev=nil, peg_rev=nil, depth=nil, &block)
..
depth ||= Core::DEPTH_INFINITY # or :infinity
...
end
# I have a plan for improving API.
# I want to provide XXX2 methods their argument is hash.
# For example, we can use proplist like the following:
# ctx.proplist2(:target => "XXX", :depth => :infinity)
> Looks like you made a change in r25016 to support use of symbols in
> the underlying implementation?
No. I just noticed and implemented that. There is no more
meaning.
Thanks,
--
kou
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [patch] ruby bindings tracking the recurse=>depth changes in svn_client_proplist3
Posted by Daniel Rall <dl...@collab.net>.
On Tue, 15 May 2007, Kouhei Sutou wrote:
> Hi,
>
> 2007/5/15, Daniel Rall <dl...@collab.net>:
> >> >- def proplist(target, rev=nil, peg_rev=nil, recurse=true, &block)
> >> >+ def proplist(target, rev=nil, peg_rev=nil,
> >> >depth=Core::DEPTH_INFINITY, &block)
> >>
> >> We can use depth=nil for the same propose. (see
> >> svn_swig_rb_to_depth()) Could you use nil instead of
> >> Svn::Core::DEPTH_INFINITY and commit this patch?
> >
> >FWIW, I like the use of an explicit value, as it makes the code more
> >easy to understand.
>
> We use depth=nil as default value in other methods.
> depth=nil means "we use default depth value" and the default
> depth value is used all depth parameter. I think using depth=nil
> for all methods is more understandable rather than using
> depth=Core::DEPTH_INFINITY only for Svn::Client#proplist.
Sure, I understand that nil equates to "use the default" -- that's a
common pattern. I don't see how nil can be claimed as more
comprehensible than an explicit value, however, when the latter is
self-documenting, and the former isn't documented at all in the
"end-user" API.
Looks like you made a change in r25016 to support use of symbols in
the underlying implementation?
Re: [patch] ruby bindings tracking the recurse=>depth changes in svn_client_proplist3
Posted by Kouhei Sutou <ko...@cozmixng.org>.
Hi,
2007/5/15, Daniel Rall <dl...@collab.net>:
> > >- def proplist(target, rev=nil, peg_rev=nil, recurse=true, &block)
> > >+ def proplist(target, rev=nil, peg_rev=nil,
> > >depth=Core::DEPTH_INFINITY, &block)
> >
> > We can use depth=nil for the same propose. (see
> > svn_swig_rb_to_depth()) Could you use nil instead of
> > Svn::Core::DEPTH_INFINITY and commit this patch?
>
> FWIW, I like the use of an explicit value, as it makes the code more
> easy to understand.
We use depth=nil as default value in other methods.
depth=nil means "we use default depth value" and the default
depth value is used all depth parameter. I think using depth=nil
for all methods is more understandable rather than using
depth=Core::DEPTH_INFINITY only for Svn::Client#proplist.
Thanks,
--
kou
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [patch] ruby bindings tracking the recurse=>depth changes in svn_client_proplist3
Posted by Daniel Rall <dl...@collab.net>.
On Tue, 15 May 2007, Kouhei Sutou wrote:
> Hi,
>
> 2007/5/15, Joe Swatosh <jo...@gmail.com>:
>
> >[[[
> >Follow on to r25007 which added depth support to svn_client_proplist3.
> >
> >* subversion/bindings/swig/ruby/svn/client.rb
> >(Svn::Client::Context#proplist) changed argument name and default to
> >reflect the use of depth instead of recurse in the core API.
> >]]]
> >
> >Index: subversion/bindings/swig/ruby/svn/client.rb
> >===================================================================
> >--- subversion/bindings/swig/ruby/svn/client.rb (revision 25014)
> >+++ subversion/bindings/swig/ruby/svn/client.rb (working copy)
> >@@ -215,7 +215,7 @@
> > # Returns list of properties attached to +target+ as an Array of
> > # Svn::Client::PropListItem.
> > # Paths and URIs are available as +target+.
> >- def proplist(target, rev=nil, peg_rev=nil, recurse=true, &block)
> >+ def proplist(target, rev=nil, peg_rev=nil,
> >depth=Core::DEPTH_INFINITY, &block)
>
> We can use depth=nil for the same propose. (see
> svn_swig_rb_to_depth()) Could you use nil instead of
> Svn::Core::DEPTH_INFINITY and commit this patch?
FWIW, I like the use of an explicit value, as it makes the code more
easy to understand.
Re: [patch] ruby bindings tracking the recurse=>depth changes in svn_client_proplist3
Posted by Joe Swatosh <jo...@gmail.com>.
Hi
On 5/14/07, Kouhei Sutou <ko...@cozmixng.org> wrote:
> Hi,
>
> 2007/5/15, Joe Swatosh <jo...@gmail.com>:
>
> > [[[
> > Follow on to r25007 which added depth support to svn_client_proplist3.
> >
> > * subversion/bindings/swig/ruby/svn/client.rb
> > (Svn::Client::Context#proplist) changed argument name and default to
> > reflect the use of depth instead of recurse in the core API.
> > ]]]
> >
> > Index: subversion/bindings/swig/ruby/svn/client.rb
> > ===================================================================
> > --- subversion/bindings/swig/ruby/svn/client.rb (revision 25014)
> > +++ subversion/bindings/swig/ruby/svn/client.rb (working copy)
> > @@ -215,7 +215,7 @@
> > # Returns list of properties attached to +target+ as an Array of
> > # Svn::Client::PropListItem.
> > # Paths and URIs are available as +target+.
> > - def proplist(target, rev=nil, peg_rev=nil, recurse=true, &block)
> > + def proplist(target, rev=nil, peg_rev=nil,
> > depth=Core::DEPTH_INFINITY, &block)
>
> We can use depth=nil for the same propose. (see
> svn_swig_rb_to_depth()) Could you use nil instead of
> Svn::Core::DEPTH_INFINITY and commit this patch?
>
> > rev ||= "HEAD"
> > peg_rev ||= rev
> > items = []
> > @@ -223,7 +223,7 @@
> > items << PropListItem.new(path, prop_hash)
> > block.call(path, prop_hash) if block
> > end
> > - Client.proplist3(target, rev, peg_rev, recurse, receiver, self)
> > + Client.proplist3(target, rev, peg_rev, depth, receiver, self)
> > items
> > end
> > alias prop_list proplist
>
>
Committed in r25019 with your modification. I figure we can change it
later if nil is the wrong default.
--
Joe
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [patch] ruby bindings tracking the recurse=>depth changes in svn_client_proplist3
Posted by Kouhei Sutou <ko...@cozmixng.org>.
Hi,
2007/5/15, Joe Swatosh <jo...@gmail.com>:
> [[[
> Follow on to r25007 which added depth support to svn_client_proplist3.
>
> * subversion/bindings/swig/ruby/svn/client.rb
> (Svn::Client::Context#proplist) changed argument name and default to
> reflect the use of depth instead of recurse in the core API.
> ]]]
>
> Index: subversion/bindings/swig/ruby/svn/client.rb
> ===================================================================
> --- subversion/bindings/swig/ruby/svn/client.rb (revision 25014)
> +++ subversion/bindings/swig/ruby/svn/client.rb (working copy)
> @@ -215,7 +215,7 @@
> # Returns list of properties attached to +target+ as an Array of
> # Svn::Client::PropListItem.
> # Paths and URIs are available as +target+.
> - def proplist(target, rev=nil, peg_rev=nil, recurse=true, &block)
> + def proplist(target, rev=nil, peg_rev=nil,
> depth=Core::DEPTH_INFINITY, &block)
We can use depth=nil for the same propose. (see
svn_swig_rb_to_depth()) Could you use nil instead of
Svn::Core::DEPTH_INFINITY and commit this patch?
> rev ||= "HEAD"
> peg_rev ||= rev
> items = []
> @@ -223,7 +223,7 @@
> items << PropListItem.new(path, prop_hash)
> block.call(path, prop_hash) if block
> end
> - Client.proplist3(target, rev, peg_rev, recurse, receiver, self)
> + Client.proplist3(target, rev, peg_rev, depth, receiver, self)
> items
> end
> alias prop_list proplist
Thanks,
--
kou
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [patch] ruby bindings tracking the recurse=>depth changes in svn_client_proplist3
Posted by Daniel Rall <dl...@collab.net>.
On Mon, 14 May 2007, Joe Swatosh wrote:
> Do we want to try to keep up with the recurse to depth changes?
Yup -- they're not subject to as much flux as the MT changes.
> [[[
> Follow on to r25007 which added depth support to svn_client_proplist3.
>
> * subversion/bindings/swig/ruby/svn/client.rb
> (Svn::Client::Context#proplist) changed argument name and default to
> reflect the use of depth instead of recurse in the core API.
> ]]]
>
> Index: subversion/bindings/swig/ruby/svn/client.rb
> ===================================================================
> --- subversion/bindings/swig/ruby/svn/client.rb (revision 25014)
> +++ subversion/bindings/swig/ruby/svn/client.rb (working copy)
> @@ -215,7 +215,7 @@
> # Returns list of properties attached to +target+ as an Array of
> # Svn::Client::PropListItem.
> # Paths and URIs are available as +target+.
> - def proplist(target, rev=nil, peg_rev=nil, recurse=true, &block)
> + def proplist(target, rev=nil, peg_rev=nil, depth=Core::DEPTH_INFINITY, &block)
> rev ||= "HEAD"
> peg_rev ||= rev
> items = []
> @@ -223,7 +223,7 @@
> items << PropListItem.new(path, prop_hash)
> block.call(path, prop_hash) if block
> end
> - Client.proplist3(target, rev, peg_rev, recurse, receiver, self)
> + Client.proplist3(target, rev, peg_rev, depth, receiver, self)
> items
> end
> alias prop_list proplist
This patch looks good, +1 to commit.