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

Hi kou,

On Nov 15, 2007 4:29 AM, Kouhei Sutou <kou@cozmixng.org> wrote:
> Hi,
>
> In <ae6cb1100711142132w2be0a09ft41893f9d2292d25c@mail.gmail.com>
> "Re: [patch] Use hash for optional args with long argument list in Ruby bindings" on Wed, 14 Nov 2007 21:32:45 -0800,
> "Joe Swatosh" <joe.swatosh@gmail.com> wrote:
>
> > I'm still not certain I understand, but I did understand that you said
> > "commit."
> > So I did, with the modification that you suggested of moving the
> > validate_options
> > method from Svn::Client::Context to Svn::Util. Now at least you can work on
> > it to show me exactly what you mean.
>
> r27828 is my answer.
> I'm sorry for my poor English skill.
>
No need to apologize. We've been doing this almost a year and really this is
the first time it has been any sort of issue. I have no Japanese at all, so
there is no way I am going to complain when I have a minor difficulty
understanding your English. Besides the diff of your commit is clear enough.

Now that I understand you were suggesting "required options" I'd like to
suggest that there are a couple renamings that could make things more clear.
And I think it would more difficult than necessary to maintain an
OPTIONAL_**_KEYS Array and a local default_options Hash. The patch below
(also attached in case gmail munges it) is only for update_editor2, but shows
what I mean.

--
Joe
Index: src-trunk/subversion/bindings/swig/ruby/svn/wc.rb
===================================================================
--- src-trunk/subversion/bindings/swig/ruby/svn/wc.rb	(revision 27830)
+++ src-trunk/subversion/bindings/swig/ruby/svn/wc.rb	(working copy)
@@ -307,47 +307,45 @@
         editor
       end
-      REQUIRED_UPDATE_EDITOR_KEYS = [:target_revision, :target]
-      OPTIONAL_UPDATE_EDITOR_KEYS = [:use_commit_times, :depth,
-                                     :allow_unver_obstruction, :diff3_cmd,
-                                     :notify_func, :cancel_func,
-                                     :traversal_info, :preserved_exts]
-      def update_editor2(options={})
-        default_options = {
+      UPDATE_EDITOR2_REQUIRED_ARGUMENTS_KEYS = [:target_revision, :target]
+      UPDATE_EDITOR2_OPTIONAL_ARGUMENTS_DEFAULTS = {
           :use_commit_times => true,
           :depth => nil,
           :allow_unver_obstruction => false,
           :diff3_cmd => nil,
           :notify_func => nil,
           :cancel_func => nil,
-          :traversal_info => _traversal_info,
+          :traversal_info => nil, # for now, really self._traversal_info()
           :preserved_exts => []
         }
-        options = default_options.merge(options)
-        Util.validate_options(options,
-                              OPTIONAL_UPDATE_EDITOR_KEYS,
-                              REQUIRED_UPDATE_EDITOR_KEYS)
+      def update_editor2(arguments={})
+        arguments = UPDATE_EDITOR2_OPTIONAL_ARGUMENTS_DEFAULTS.
+                          merge(:traversal_info => _traversal_info).
+                          merge(arguments)
+        Util.validate_options(arguments,
+                              UPDATE_EDITOR2_OPTIONAL_ARGUMENTS_DEFAULTS.keys,
+                              UPDATE_EDITOR2_REQUIRED_ARGUMENTS_KEYS)
         # TODO(rb support fetch_fun): implement support for the fetch_func
         # callback.
-        options[:fetch_func] = nil
+        arguments[:fetch_func] = nil
         # TODO(rb support conflict_fun): implement support for the
         # conflict_func callback.
-        options[:conflict_func] = nil
+        arguments[:conflict_func] = nil
-        results = Wc.get_update_editor3(options[:target_revision], self,
-                                        options[:target],
-                                        options[:use_commit_times],
-                                        options[:depth],
-                                        options[:allow_unver_obstruction],
-                                        options[:notify_func],
-                                        options[:cancel_func],
-                                        options[:conflict_func],
-                                        options[:fetch_func],
-                                        options[:diff3_cmd],
-                                        options[:preserved_exts],
-                                        options[:traversal_info])
+        results = Wc.get_update_editor3(arguments[:target_revision], self,
+                                        arguments[:target],
+                                        arguments[:use_commit_times],
+                                        arguments[:depth],
+                                        arguments[:allow_unver_obstruction],
+                                        arguments[:notify_func],
+                                        arguments[:cancel_func],
+                                        arguments[:conflict_func],
+                                        arguments[:fetch_func],
+                                        arguments[:diff3_cmd],
+                                        arguments[:preserved_exts],
+                                        arguments[:traversal_info])
         target_revision_address, editor, editor_baton = results
         editor.__send__(:target_revision_address=, target_revision_address)
         editor.baton = editor_baton

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