Re: [patch] Use hash for optional args with long argument list in Ruby bindings

Hi

On Nov 13, 2007 3:54 AM, Kouhei Sutou <kou@cozmixng.org> wrote:
> Hi,
>
> In <ae6cb1100711121923yeb154f8ib3975e34d21a9281@mail.gmail.com>
> "[patch] Use hash for optional args with long argument list in Ruby bindings" on Mon, 12 Nov 2007 19:23:13 -0800,
> "Joe Swatosh" <joe.swatosh@gmail.com> wrote:
>
> > Although I didn't modify Hash for checking like the example from ActiveSupport
> > that you showed. It didn't seem right to modify a standard class from the SVN
> > bindings.
>
> OK. I agree with you.
> I want to define them in Svn::Util.

Should we create a new file in svn called util.rb for the module?
Then just include it in Context?

>
> > Here's hoping this is more like what you had in mind.
>
> Please commit but I have some comments.
>
> > Index: subversion/bindings/swig/ruby/svn/wc.rb
> > ===================================================================
> > --- subversion/bindings/swig/ruby/svn/wc.rb (revision 27776)
> > +++ subversion/bindings/swig/ruby/svn/wc.rb (working copy)
> > @@ -307,25 +307,37 @@
> > editor
> > end
> >
> > - def update_editor2(target_revision, target, use_commit_times=true,
> > - depth=nil, allow_unver_obstruction=false, diff3_cmd=nil,
> > - notify_func=nil, cancel_func=nil, traversal_info=nil,
> > - preserved_exts=nil)
> > - preserved_exts ||= []
> > - traversal_info ||= _traversal_info
> > + def update_editor2(target_revision, target, options={})
>
> We doesn't need to keep the current update_editor2 API
> because 1.4 doesn't have update_editor2 API. If you want to
> keep the current update_editor2 API, what about the
> following?

Since Svn::Client::Context#update_editor2 doesn't exist in 1.4, I wasn't trying
to keep the long argument list version around. Although, rereading the code
above makes me wonder if target_revision shouldn't be made optional too.

Okay, I'm feeling thick this morning the the fragments below aren't making much
sense to me. Do you mean "# the current updated_editor2 API" below? Are
you suggesting keeping the many optional argument version? If so, why don't we
leave Svn::Client::Context#update_editor2 alone and add a
Svn::Client::Context#update_editor3 that uses the Hash arguments?

>
> def update_editor2(*args, &block)
> update_editor3(*args, &block) if args.size != 1 and !args.is_a?(Hash)
> options = args[0]
> ... # your code
> end
>
> def update_editor3(target_revision, target, use_commit_times, ...)
> # the current update_editor3 API
> end
>
> > - def switch_editor2(target_revision, target, switch_url,
> > - use_commit_times=true, depth=nil,
> > - allow_unver_obstruction=false, diff3_cmd=nil,
> > - notify_func=nil, cancel_func=nil, traversal_info=nil,
> > - preserved_exts=nil)
> > + def switch_editor2(target_revision, target, switch_url, options={})
>
> Same as update_editor2.
>
>
> Thanks,
> --
> kou
>

Thanks

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