You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Jens Seidel <je...@users.sourceforge.net> on 2008/06/10 08:10:58 UTC

--diff-cmd doesn't use -u per default

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@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: --diff-cmd doesn't use -u per default

Posted by "C. Michael Pilato" <cm...@collab.net>.
Julian Foad wrote:
> On Tue, 2008-06-10 at 10:59 -0400, C. Michael Pilato wrote:
>> I seem to be doing alot of talking to myself at the moment, but that's cool.
> 
> I'm listening. Sorry that you didn't read my mind and know that's what I
> was doing. :-)
> 
>> For the record, I think the correct-er of the fixes is the second patch. 
> 
> Your patch 1 (send user_args==NULL rather than uninitialised) is
> essential to fix this regression.

Okey dokey.

> Your patch 2 ((svn_io_run_diff): Don't check user_args for NULL-ness;
> consult the num_user_args instead) is bad: it will break this public API
> for clients that were relying on it working the way it did work, and
> initialising only 'user_args' but not 'num_user_args'. (Note also that
> the test for null occurs twice in the function and your patch changes
> only one occurrence.)

Ah, thanks.

> FWIW, it looks to me like this bug was introduced in r30053 in the
> calling code. The svn_io_run_diff() API has been stable for ages.
> 
> The correct fix should include:
> 
> * Your patch 1.
> 
> * Update the doc string for svn_io_run_diff() to match the
>   implementation, which is to say that user_args must be NULL to
>   indicate no user args specified and thus get the default "-u".
>   (Presently it's ambiguous on whether user_args need be NULL
>   and/or num_user_args need be 0 to get that behaviour.)

Alright.  I've some time, so I'll go this route.

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


Re: --diff-cmd doesn't use -u per default

Posted by Lieven Govaerts <sv...@mobsol.be>.
Julian Foad wrote:
> On Tue, 2008-06-10 at 21:10 +0200, Lieven Govaerts wrote:
>   
>>>> You obviously meant:
>>>>  3. A regression test, as this issue is important enough to hold off 
>>>> the 10th RC and almost caused a regression.
>>>>
>>>> Lieven - always prepared to provide test writing cycles.
>>>>         
>>> The fix is in -- feel free to start your test composition now.
>>>
>>>       
>> r31688. This one wasn't actually that hard, at least not compared to 
>> some of the merge tests.
>>     
>
> Thanks!
>
> (Do we need to back-port tests? You mentioned doing so in IRC, but tests
> are not in themselves critical to making the branch suitable for
> release.)
>
>   
Normally we should. Having the test only on trunk is not really useful, 
as the test might pass on trunk but still fail on the branch. I normally 
add the test to the group of revisions for the fix, but here the fix was 
approved before I could add it, so I'll just add the test revision 
separately.

Lieven



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

Re: --diff-cmd doesn't use -u per default

Posted by Julian Foad <ju...@btopenworld.com>.
On Tue, 2008-06-10 at 21:10 +0200, Lieven Govaerts wrote:
> >> You obviously meant:
> >>  3. A regression test, as this issue is important enough to hold off 
> >> the 10th RC and almost caused a regression.
> >>
> >> Lieven - always prepared to provide test writing cycles.
> >
> > The fix is in -- feel free to start your test composition now.
> >
> r31688. This one wasn't actually that hard, at least not compared to 
> some of the merge tests.

Thanks!

(Do we need to back-port tests? You mentioned doing so in IRC, but tests
are not in themselves critical to making the branch suitable for
release.)

- Julian



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

Re: --diff-cmd doesn't use -u per default

Posted by Lieven Govaerts <sv...@mobsol.be>.
C. Michael Pilato wrote:
> Lieven Govaerts wrote:
>> Julian Foad wrote:
>>> On Tue, 2008-06-10 at 10:59 -0400, C. Michael Pilato wrote:
>>>> I seem to be doing alot of talking to myself at the moment, but 
>>>> that's cool.
>>>
>>> I'm listening. Sorry that you didn't read my mind and know that's 
>>> what I
>>> was doing. :-)
>>>
>>>> For the record, I think the correct-er of the fixes is the second 
>>>> patch. 
>>>
>>> Your patch 1 (send user_args==NULL rather than uninitialised) is
>>> essential to fix this regression.
>>>
>>> Your patch 2 ((svn_io_run_diff): Don't check user_args for NULL-ness;
>>> consult the num_user_args instead) is bad: it will break this public 
>>> API
>>> for clients that were relying on it working the way it did work, and
>>> initialising only 'user_args' but not 'num_user_args'. (Note also that
>>> the test for null occurs twice in the function and your patch changes
>>> only one occurrence.)
>>>
>>> FWIW, it looks to me like this bug was introduced in r30053 in the
>>> calling code. The svn_io_run_diff() API has been stable for ages.
>>>
>>> The correct fix should include:
>>>
>>> * Your patch 1.
>>>
>>> * Update the doc string for svn_io_run_diff() to match the
>>>   implementation, which is to say that user_args must be NULL to
>>>   indicate no user args specified and thus get the default "-u".
>>>   (Presently it's ambiguous on whether user_args need be NULL
>>>   and/or num_user_args need be 0 to get that behaviour.)
>>>
>> You obviously meant:
>>  3. A regression test, as this issue is important enough to hold off 
>> the 10th RC and almost caused a regression.
>>
>> Lieven - always prepared to provide test writing cycles.
>
> The fix is in -- feel free to start your test composition now.
>
r31688. This one wasn't actually that hard, at least not compared to 
some of the merge tests.

Lieven

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

Re: --diff-cmd doesn't use -u per default

Posted by "C. Michael Pilato" <cm...@collab.net>.
Lieven Govaerts wrote:
> Julian Foad wrote:
>> On Tue, 2008-06-10 at 10:59 -0400, C. Michael Pilato wrote:
>>> I seem to be doing alot of talking to myself at the moment, but 
>>> that's cool.
>>
>> I'm listening. Sorry that you didn't read my mind and know that's what I
>> was doing. :-)
>>
>>> For the record, I think the correct-er of the fixes is the second patch. 
>>
>> Your patch 1 (send user_args==NULL rather than uninitialised) is
>> essential to fix this regression.
>>
>> Your patch 2 ((svn_io_run_diff): Don't check user_args for NULL-ness;
>> consult the num_user_args instead) is bad: it will break this public API
>> for clients that were relying on it working the way it did work, and
>> initialising only 'user_args' but not 'num_user_args'. (Note also that
>> the test for null occurs twice in the function and your patch changes
>> only one occurrence.)
>>
>> FWIW, it looks to me like this bug was introduced in r30053 in the
>> calling code. The svn_io_run_diff() API has been stable for ages.
>>
>> The correct fix should include:
>>
>> * Your patch 1.
>>
>> * Update the doc string for svn_io_run_diff() to match the
>>   implementation, which is to say that user_args must be NULL to
>>   indicate no user args specified and thus get the default "-u".
>>   (Presently it's ambiguous on whether user_args need be NULL
>>   and/or num_user_args need be 0 to get that behaviour.)
>>
> You obviously meant:
>  3. A regression test, as this issue is important enough to hold off the 
> 10th RC and almost caused a regression.
> 
> Lieven - always prepared to provide test writing cycles.

The fix is in -- feel free to start your test composition now.

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


Re: --diff-cmd doesn't use -u per default

Posted by Lieven Govaerts <sv...@mobsol.be>.
Julian Foad wrote:
> On Tue, 2008-06-10 at 10:59 -0400, C. Michael Pilato wrote:
>> I seem to be doing alot of talking to myself at the moment, but that's cool.
> 
> I'm listening. Sorry that you didn't read my mind and know that's what I
> was doing. :-)
> 
>> For the record, I think the correct-er of the fixes is the second patch. 
> 
> Your patch 1 (send user_args==NULL rather than uninitialised) is
> essential to fix this regression.
> 
> Your patch 2 ((svn_io_run_diff): Don't check user_args for NULL-ness;
> consult the num_user_args instead) is bad: it will break this public API
> for clients that were relying on it working the way it did work, and
> initialising only 'user_args' but not 'num_user_args'. (Note also that
> the test for null occurs twice in the function and your patch changes
> only one occurrence.)
> 
> FWIW, it looks to me like this bug was introduced in r30053 in the
> calling code. The svn_io_run_diff() API has been stable for ages.
> 
> The correct fix should include:
> 
> * Your patch 1.
> 
> * Update the doc string for svn_io_run_diff() to match the
>   implementation, which is to say that user_args must be NULL to
>   indicate no user args specified and thus get the default "-u".
>   (Presently it's ambiguous on whether user_args need be NULL
>   and/or num_user_args need be 0 to get that behaviour.)
> 
You obviously meant:
  3. A regression test, as this issue is important enough to hold off 
the 10th RC and almost caused a regression.

Lieven - always prepared to provide test writing cycles.



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

Re: --diff-cmd doesn't use -u per default

Posted by Julian Foad <ju...@btopenworld.com>.
On Tue, 2008-06-10 at 10:59 -0400, C. Michael Pilato wrote:
> I seem to be doing alot of talking to myself at the moment, but that's cool.

I'm listening. Sorry that you didn't read my mind and know that's what I
was doing. :-)

> For the record, I think the correct-er of the fixes is the second patch. 

Your patch 1 (send user_args==NULL rather than uninitialised) is
essential to fix this regression.

Your patch 2 ((svn_io_run_diff): Don't check user_args for NULL-ness;
consult the num_user_args instead) is bad: it will break this public API
for clients that were relying on it working the way it did work, and
initialising only 'user_args' but not 'num_user_args'. (Note also that
the test for null occurs twice in the function and your patch changes
only one occurrence.)

FWIW, it looks to me like this bug was introduced in r30053 in the
calling code. The svn_io_run_diff() API has been stable for ages.

The correct fix should include:

* Your patch 1.

* Update the doc string for svn_io_run_diff() to match the
  implementation, which is to say that user_args must be NULL to
  indicate no user args specified and thus get the default "-u".
  (Presently it's ambiguous on whether user_args need be NULL
  and/or num_user_args need be 0 to get that behaviour.)


> 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

Yes, that's goodness, but something to consider for later and perhaps
hardly worth doing until we re-vamp this external diff support properly.

> 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.


- Julian



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

Re: --diff-cmd doesn't use -u per default

Posted by "C. Michael Pilato" <cm...@collab.net>.
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@subversion.tigris.org
>>> For additional commands, e-mail: dev-help@subversion.tigris.org
>>>
>>
>>
> 
> 


-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


Re: --diff-cmd doesn't use -u per default

Posted by "C. Michael Pilato" <cm...@collab.net>.
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@subversion.tigris.org
>> For additional commands, e-mail: dev-help@subversion.tigris.org
>>
> 
> 


-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand

Re: --diff-cmd doesn't use -u per default

Posted by "C. Michael Pilato" <cm...@collab.net>.
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@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
> 


-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


Re: --diff-cmd doesn't use -u per default

Posted by Erik Huelsmann <eh...@gmail.com>.
On 6/10/08, Julian Foad <ju...@btopenworld.com> wrote:
> On Tue, 2008-06-10 at 15:30 +0200, Erik Huelsmann wrote:
> > On 6/10/08, Jens Seidel <je...@users.sf.net> wrote:
> > > I also wonder why a temporary copy is provided to the diff command. This
> > > is probably required if the comparision is not against BASE.
> > > Nevertheless I like to edit the file in the diff viewer as well and have
> > > to parse option 2 or 4.
>
> I, too, fairly often want to edit my working file while it's in my diff
> viewer, and I can't usefully do so when it's a temporary copy.
>
> > Because you need to de-translate keywords and line-endings in order to
> > get a clean diff between the working copy file and the base.
>
> Well, you're right in as much as that we need to translate these things
> in some direction to get a clean diff, but need we do it that way round?
>
> Instead of giving a diff between (base or something like it) and (how
> the working copy will be committed to the repository), I can see
> arguments for giving a diff between (what the fresh working copy would
> have been) and (what the working copy is now). That would provide the
> user's own line endings, and his diff tool might work better with them
> than with the repository-normal-format line endings that we currently
> give it when eol-style=native (or do we? I'm not sure). It would also
> show any changes he might have made accidentally or uncomprehendingly to
> the keywords. No doubt there are opposing arguments too, but this seems
> to have some merit.

Absolutely. I don't think we have an outstanding request to this
extent, but it's definitely not the first time we're seeing this
request. The only problem with it that it will also show differences
in line-endings which would normally be translated away upon commit.

Other than that, I think this is probably a 3 or 4 line change.

Bye,

Erik.

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

Re: --diff-cmd doesn't use -u per default

Posted by Julian Foad <ju...@btopenworld.com>.
On Tue, 2008-06-10 at 15:30 +0200, Erik Huelsmann wrote:
> On 6/10/08, Jens Seidel <je...@users.sf.net> wrote:
> > I also wonder why a temporary copy is provided to the diff command. This
> > is probably required if the comparision is not against BASE.
> > Nevertheless I like to edit the file in the diff viewer as well and have
> > to parse option 2 or 4.

I, too, fairly often want to edit my working file while it's in my diff
viewer, and I can't usefully do so when it's a temporary copy.

> Because you need to de-translate keywords and line-endings in order to
> get a clean diff between the working copy file and the base.

Well, you're right in as much as that we need to translate these things
in some direction to get a clean diff, but need we do it that way round?

Instead of giving a diff between (base or something like it) and (how
the working copy will be committed to the repository), I can see
arguments for giving a diff between (what the fresh working copy would
have been) and (what the working copy is now). That would provide the
user's own line endings, and his diff tool might work better with them
than with the repository-normal-format line endings that we currently
give it when eol-style=native (or do we? I'm not sure). It would also
show any changes he might have made accidentally or uncomprehendingly to
the keywords. No doubt there are opposing arguments too, but this seems
to have some merit.

- Julian



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

Re: --diff-cmd doesn't use -u per default

Posted by Erik Huelsmann <eh...@gmail.com>.
On 6/10/08, Jens Seidel <je...@users.sf.net> wrote:
> On Tue, Jun 10, 2008 at 10:10:58AM +0200, Jens Seidel wrote:
> > 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.
>
> I also wonder why a temporary copy is provided to the diff command. This
> is probably required if the comparision is not against BASE.
> Nevertheless I like to edit the file in the diff viewer as well and have
> to parse option 2 or 4.

Because you need to de-translate keywords and line-endings in order to
get a clean diff between the working copy file and the base.

HTH,

Erik.

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

Re: --diff-cmd doesn't use -u per default

Posted by Jens Seidel <je...@users.sourceforge.net>.
On Tue, Jun 10, 2008 at 12:13:40PM +0200, Branko Čibej wrote:
> Branko Čibej wrote:
> >Jens Seidel wrote:
> >>LEFT=$(echo "$LEFT" | sed 's/[\t ]([^()]*)//')
> >>vimdiff "+set wrap" "+wincmd l" "+set wrap" "+wincmd h" $LEFT $RIGHT
> >>  
> >
> >That's just wrong. The contents of $6 and your computed $LEFT are not 
> >necessarily the same.

???

I didn't even used $6! If you know the argument processing please submit
a patch against the svn book. It is currently wrong about it (uses ${6}
and ${7}).

http://svnbook.red-bean.com/nightly/en/svn-book.html#svn.advanced.externaldifftools

> (And, I might add, your question belongs on the users@ list :-P  )

That may be right for the second part but not for the documentation bug
in svn help diff. This needs to be fixed in 1.5.x. If the help is right,
the subversion code needs to be changed ...

Jens

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

Re: --diff-cmd doesn't use -u per default

Posted by Branko Čibej <br...@xbc.nu>.
Branko Čibej wrote:
> Jens Seidel wrote:
>> LEFT=$(echo "$LEFT" | sed 's/[\t ]([^()]*)//')
>> vimdiff "+set wrap" "+wincmd l" "+set wrap" "+wincmd h" $LEFT $RIGHT
>>   
>
> That's just wrong. The contents of $6 and your computed $LEFT are not 
> necessarily the same.

(And, I might add, your question belongs on the users@ list :-P  )


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

Re: --diff-cmd doesn't use -u per default

Posted by Branko Čibej <br...@xbc.nu>.
Jens Seidel wrote:
> On Tue, Jun 10, 2008 at 10:10:58AM +0200, Jens Seidel wrote:
>   
>> 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.
>>     
>
> I also wonder why a temporary copy is provided to the diff command. This
> is probably required if the comparision is not against BASE.
> Nevertheless I like to edit the file in the diff viewer as well and have
> to parse option 2 or 4.
>
> Current parameters:
> #1=-L
> #2=../src/file.c    (Revision 1584)
> #3=-L
> #4=../src/file.c    (Arbeitskopie)
> #5=../src/.svn/text-base/file.c.svn-base
> #6=/tmp/svndiff.3.tmp
> #7=
> #8=
>
> my script:
>
> #!/bin/sh
> LEFT=${4}
> RIGHT=${5}
>
> LEFT=$(echo "$LEFT" | sed 's/[\t ]([^()]*)//')
> vimdiff "+set wrap" "+wincmd l" "+set wrap" "+wincmd h" $LEFT $RIGHT
>   

That's just wrong. The contents of $6 and your computed $LEFT are not 
necessarily the same.

-- Brane


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

Re: --diff-cmd doesn't use -u per default

Posted by Jens Seidel <je...@users.sourceforge.net>.
On Tue, Jun 10, 2008 at 10:10:58AM +0200, Jens Seidel wrote:
> 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.

I also wonder why a temporary copy is provided to the diff command. This
is probably required if the comparision is not against BASE.
Nevertheless I like to edit the file in the diff viewer as well and have
to parse option 2 or 4.

Current parameters:
#1=-L
#2=../src/file.c    (Revision 1584)
#3=-L
#4=../src/file.c    (Arbeitskopie)
#5=../src/.svn/text-base/file.c.svn-base
#6=/tmp/svndiff.3.tmp
#7=
#8=

my script:

#!/bin/sh
LEFT=${4}
RIGHT=${5}

LEFT=$(echo "$LEFT" | sed 's/[\t ]([^()]*)//')
vimdiff "+set wrap" "+wincmd l" "+set wrap" "+wincmd h" $LEFT $RIGHT

Jens

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