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

Hi kou,

On Nov 17, 2007 10:03 PM, Kouhei Sutou <kou@cozmixng.org> wrote:
> Hi,
>
> In <ae6cb1100711172107g2e3b6d57ufac1aa0723dc97d8@mail.gmail.com>
> "Re: [patch] Use hash for optional args with long argument list in Ruby bindings" on Sat, 17 Nov 2007 21:07:30 -0800,
> "Joe Swatosh" <joe.swatosh@gmail.com> wrote:
>

>
> Anyway, the 1.4 codes has bugs (I passed wrong arguments to
> get_update_editor2) and I fixed in r24302.
>
> For now, update_editor codes is based on 1.4 and has the
> bugs. We need to fix the bugs like I did in r24302. I'll
> attach a patch for update_editor. switch_editor will need to
> be fixed.
>

Okay, I'm completely missing the boat here. I guess I'm still not convinced
we need to support update_editor and switch_editor (the compatibility
versions) since they have bugs. I'm starting to think I shouldn't have messed
with this at all....

Where we are now:

  A) update_editor2 and switch_editor2 are correct except that they have an
     unnecessary required argument (:target_revision) that should be an
     optional argument that your patch fixes for update_editor2.

  B) update_editor and switch_editor are copy-pasted from 1.4 bugs and all.
     r24302 which addressed the bugs on trunk also modified the calling
     interfaces so can't be directly applied to fix the bugs.

Where we want to be(?):

  C) update_editor2 and switch editor2 need to have :target_revision moved
     from required to optional and apply the revnum defaulting logic to
     svn_wc.i (as the patch)

  D) update_editor and switch_editor fix the bugs, but leave the calling
     interface the same.

I guess I'm questioning the value of trying to maintain any sort of
compatibility with 1.4 for update_editor and switch_editor since they are so
broken in the branch. Perhaps we should just revert back to where they were
before I copy-pasted from 1.4 and try to move forward from there?

I will apply the parts of the patch that do C) above, plus move
:target_revision from required to optional for switch_editor2. I'll leave the
rest to you, if that's okay.

I'm still trying to figure out how to test the :conflict_func. I can make a
conflicted wc simply enough, but I guess I don't understand well enough when
the callback is supposed to be invoked.

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