Hmm, I understand the concern about using the empty string to mean "internal
diff".
Here's my concerns with the other UI of having a --force-internal-diff.
These may be handled by svn's options parser library, and thus response of
the form "go read this code" is totally understandable, but if you could
tell me what code to read I'd be grateful.
What happens if I set --force-internal-diff while also setting
--diff-cmd="/path/to/my/diff"?
What happens if I set force-internal-diff in my config, but diff-cmd on the
command line?
It seems to me what we are discussing is one option (which diff command
should be used), and so it makes sense to specify that option in one place.
Using "" to mean "built-in" may be confusing. I get that. Another option
would be to be "builtin" to mean "built-in", but that 1) prevents running
binaries named "builtin" and 2) is easy to confuse with something else; say
"built-in".
Having "" mean default matches informal user expectation, I posit.
Uninitialized strings in languages that have a default are "", and supply
the value most like null to specify that you meant no value seems
consistent.
I'd love to do what will make this work, but I think that keeping it as one
option (especially when it's documented that "" is a special value instead
of a non-sensical one to specify) is going to be simplest when integrated
over many variables. The discussion on the bug (
http://subversion.tigris.org/issues/show_bug.cgi?id=3071 ) settled with jph
saying this wasn't a perfect syntax, but acceptable. It's not perfect, but
I think it might be the least flawed out there.
Thank you for the help/guidance,
-Dan
On Mon, Mar 24, 2008 at 4:33 PM, C. Michael Pilato <cmpilato_at_collab.net>
wrote:
> Stefan Sperling wrote:
> > On Mon, Mar 24, 2008 at 05:05:06AM -0400, Daniel Danger Bentley wrote:
> >> Ping.
> >>
> >> Is there something I can do to help this process along or that I
> forgot to
> >> do initially? Should I pick a reviewer based on revision history?
> >
> > Since no-one else has commented so far, let me just give you my
> > (non-authoritative!) take on the design of your patch.
> >
> > I think making an empty string carry special semantics is wrong UI
> design.
> > It's not obvious enough.
> >
> > What would be better IMHO (and various standard UNIX tools do the
> > same thing) is adding an additional option that overrides all previous
> > --diff-cmd options, and also config file settings.
> >
> > Maybe --force-internal-diff ?
>
> As a commentary about the proposed UI, I have to agree with Stefan here.
>
> --
> C. Michael Pilato <cmpilato_at_collab.net>
> CollabNet <> www.collab.net <> Distributed Development On Demand
>
>
--
'Ladislav Sticha, the tall spokesman for Czech Television, told me that the
show's audience was "miniature" — presumably he meant small in number.' -
New York Times, January 24, 2008
Received on 2008-03-25 08:29:34 CET