I seem to be doing alot of talking to myself at the moment, but that's cool.
For the record, I think the correct-er of the fixes is the second patch.
But the super-duper-bestest fix would be to rev the svn_io_run_diff()
interface and accept apr_array_header_t *user_args instead of a C memory
array and length indicator. Doing so allows us to support three codepaths:
a. NULL array: provide the default '-u' parameter to the external diff
b. empty array: provide no parameters
c. non-empty array: provide the caller's specified parameters
We can, of course, do this today, but folks wishing to use codepath (b) are
forced to pass some icky non-NULL value for the C array parameter. That's
nasty, so my second patch nipped that mess in the bud.
C. Michael Pilato wrote:
> By the way, I haven't looked closely, but I'm betting that the problem
> is the difference between a NULL user_args array and an empty one
> getting passed into svn_io_run_diff().
>
> Attached are two untested patches, each of which probably fixes the
> problem, but in different ways.
>
>
>
> C. Michael Pilato wrote:
>> Jens, using 1.5.x, I can confirm that the two files to diff have
>> shifted to args 5 and 6 (instead of 6 and 7) of the external diff
>> program, and that '-u' is no longer provided to that program.
>>
>> So, yeah, looks like a bug, to me.
>>
>> And unfortunately, I believe our diff-cmd interface is a form of API,
>> and that this constitutes an API compatability violation.
>>
>>
>> Jens Seidel wrote:
>>> Hi,
>>>
>>> I noticed that I had to rewrite my little vimdiff wrapper for svn diff
>>> again for 1.5.x. In the past -u was provided as first argument, now it
>>> isn't.
>>>
>>> $ svn help diff
>>> --diff-cmd ARG : use ARG as diff command
>>> -x [--extensions] ARG : Default: '-u'. When Subversion is
>>> invoking an
>>> external diff program, ARG is simply
>>> passed along
>>> to the program. But when Subversion is
>>> using its
>>> default internal diff implementation, or
>>> when
>>> Subversion is displaying blame
>>> annotations, ARG
>>> could be any of the following:
>>> -u (--unified):
>>> Output 3 lines of unified context.
>>>
>>>
>>> $ svn diff --diff-cmd diff
>>> Index: subversion/po/de.po
>>> ===================================================================
>>> 9432d9431
>>> < # CHECKME: " :"?
>>> 9440c9439
>>> < "A)bbrechen, Weitermac)hen, E)ditieren :\n"
>>> ---
>>>> "A)bbrechen, Weitermac)hen, E)ditieren:\n"
>>>
>>> Is this output style intented? If yes, it should maybe also mentioned
>>> in the svnbook.
>>>
>>> Jens
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
>>> For additional commands, e-mail: dev-help_at_subversion.tigris.org
>>>
>>
>>
>
>
--
C. Michael Pilato <cmpilato_at_collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Received on 2008-06-10 17:00:06 CEST