Gabriela Gibson wrote on Wed, Jun 12, 2013 at 14:18:08 +0100:
> On 6/11/13, Daniel Shahaf <danielsh_at_elego.de> wrote:
> > Gabriela Gibson wrote on Mon, Jun 10, 2013 at 23:39:45 +0100:
> >> On 6/10/13, Daniel Shahaf <danielsh_at_elego.de> wrote:
> >>
> >> * @a invoke_diff_cmd takes an argument which is used to call an
> >> * external diff program. When invoked, the argument may not be @c
> >> * NULL or the empty string "". The command line invocation will
> >> * override the invoke-diff-cmd invocation entry (if any) in the
> >> * Subversion configuration file.
> >>
> >> Is that passable?
> >>
> >
> > No, because the argument _may_ be NULL (since the deprecated code path
> > (and people updating calls to deprecated functions in their code to call
> > the non-deprecated equivalents) needs it), and the docstring says it may
> > not be. Note, even people who updated their code to the 1.9 API might
> > still want to pass NULL because they don't want to override the
> > invoke-diff-cmd in the config (or because they want to use the builtin
> > diff implementation).
> >
> > Does this make sense?
>
> I had a look how TortoiseSVN approaches this. From what I can
> see, no matter what we do, they have to adjust their code, because
> they turn off the config file options explicitly when they run
> their custom code, so adding invoke-diff-cmd to the config file
> will scupper that plan[1]
>
Our usual argument is "This only affects users who use the new features
(the new invoke-diff-cmd config file option)". I'm not sure what's the
best solution here. How should we handle a client written against 1.7
libraries and run against 1.8 libraries?
Should the deprecated wrapper have the invoke-diff-cmd entry in
ctx->config disabled? Are there other options?
> So in that sense, behavior *has* changed and our addition is not
> backward compatible with older clients, even if they use ...diff6().
>
> I think I should probably write a short intro to the new code for
> client devs to make updating their code easier. Another thing to
> consider is whether we should offer a 'switch service' in to make
> external surgery of the set config file values unnecessary.
>
What's a "switch service"? Is that just svn_client_diff6() ?
> On the bright side, I've finally understood why it's OK to pass
> NULL safely because the code path is selected on that value. What
> I should have said in the doxygen comment to
> svn_io_create_custom_diff_cmd is that thou shall not pass NULL.
>
> New comment for svn_client_diff7 is now:
>
> * @a invoke_diff_cmd takes an argument which is used to call an
> * external diff program when not NULL or "".
>
> (Will also update the svn_io_create_custom_diff_cmd doxygen comment.)
> ==================================================================
> About the delimiter to select:
>
> I don't have the expertise to make an informed decisions here, or
> even to hold a strong opinion. But I happily code whatever the
> team chooses :) I could also rewrite (or try to!) the
> svn_io_create_custom_diff_cmd in C printf style if the current
> code shape is the wrong approach for what we want to do (it
> probably is)
>
> I still think the ---f1 pattern is the easiest to use because
> it's guaranteed not to interfere with anything, nothing needs to
> be escaped either and everything users want to do can be done
> much simpler.
>
> The strongest argument against ---f1 was it's novel and a bit
> ugly, but, in the scheme of things, I think this is a smaller
> problem than overlaying user's interpolation schemes and
> painstakingly counting and escaping %'s or $'s.
>
> Also it visually separates the user's interpolation scheme nicely
> from ours.
>
> It's no problem to allow users the choice of two interpolating
> syntaxes either, maybe the breadths of possibilities is just to
> broad to make a decent swiss army knife here that covers
> everyone's needs?
>
Let's not add choices if we don't have to. One interpolation syntax is
perfectly fine. Just pick one, specify it, implement it. The
considerations unambiguity of the syntax specification, ease of
implementation, and clarity to users.
(I'm sure someone will add considerations that I missed, if any.)
I know we already went through the "what syntax to choose", I hope you
aren't getting frustrated by going through it a second time. We're just
trying to pick a good, unambiguous syntax.
Cheers,
Dainel
Received on 2013-06-14 12:32:26 CEST