You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Frederic Melot <fr...@lpsc.in2p3.fr> on 2008/11/13 10:37:29 UTC
Re: [PATCH] issue #3292: 'svnlook diff' regression: doesn't print
added dirs
Hello,
yes by metadata I mean 'properties'.
In fact at the very beginning I reported a regression from 1.4.x to
1.5.x. It was only on the copied folder. Added folders (without history)
was not triggered by svnlook in svn 1.4.x.
In issue #3292 added folder are included. OK, but what about renamed and
deleted folders, which are not triggered?
My suggestion is to add them. What's your opinion?
Best regards,
Frédéric
Daniel Shahaf wrote:
> Frederic Melot wrote on Fri, 31 Oct 2008 at 17:57 +0100:
>
>> Hello,
>>
>> you are totally right. My first concern was only on the "Copied:" headers
>> for directories and I didn't change anything for "Added:" directories. I'll
>> do it.
>>
>>
>
> Okay. Note that there are a few other variants (e.g. whether the add is
> of a file or of a dir, whether it is with/without history) -- watch your
> step, then, don't break one while fixing the other ;)
>
>
>> For the second point, I totally agree. But the code of the
>> 'print_diff_tree' function is mainly triggerring 3 cases:
>> 1) the copied files or folders, which update the string buffer and doesn't
>> print it.
>> 2) the files modifications, which update the string buffer and display it.
>> It is the only code part in which the string buffer is displayed. This is
>> why diffs on folders never appear.
>> 3) the meta data modifications, a part which does not make use of the
>>
>
> What is "meta data"? Did you mean 'properties'?
>
>
>> string buffer and prints directly the results
>> As the string buffer was already not fully used I didn't use it. If we want
>> to use it more, this function needs to be more deeper modified. Is it what
>> you suggested?
>>
>>
>
> I haven't actually suggested anything yet :)
>
> The problem isn't that the stringbuf is underused; it is that its
> accounting needs some fixes. Currently, sometimes the stringbuf is filled
> but isn't printed; with your (previous) patch, it was not obvious that
> there weren't cases where (parts of) the stringbuf would be printed twice.
>
> Whether solving the accounting problem should be done by increasing the
> use of stringbufs I leave for you to decide...
>
> Thanks,
>
> Daniel
>
>
>> Best regards,
>> Frederic
>>
>> Daniel Shahaf wrote:
>>
>>> Frederic Melot wrote on Tue, 21 Oct 2008 at 16:48 +0200:
>>>
>>>
>>>> Hello,
>>>>
>>>> here is a patch submission for issue #3292.
>>>>
>>>> Best regards,
>>>> Frédéric Melot
>>>>
>>>>
>>>>
>>> Frédéric, thanks for the patch. I have now tried it, and it still
>>> doesn't print the "Added:" headers for added-without-history
>>> directories. Could you look into that?
>>>
>>> Also, I'm worried that printing the header right away (as you did) could
>>> lead to other problems: for example, how do you ensure that the header
>>> isn't printed twice (in any case)? That code already isn't the easiest
>>> to follow, and adding prints randomly scattered through won't make it
>>> better.
>>>
>>> Thanks,
>>>
>>> Daniel
>>> ------------------------------------------------------------------------
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
>>> For additional commands, e-mail: dev-help@subversion.tigris.org
>>>
Re: [PATCH] issue #3292: 'svnlook diff' regression: doesn't print
added dirs
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Frederic Melot wrote on Thu, 13 Nov 2008 at 11:37 +0100:
> Hello,
>
> yes by metadata I mean 'properties'.
>
> In fact at the very beginning I reported a regression from 1.4.x to 1.5.x.
> It was only on the copied folder. Added folders (without history) was not
> triggered by svnlook in svn 1.4.x.
>
(I'm assuming "triggered" means "printed to the output of 'svnlook diff'")
> In issue #3292 added folder are included. OK, but what about renamed and
> deleted folders, which are not triggered?
>
> My suggestion is to add them. What's your opinion?
>
If we already print 'Added:', it IMO makes sense to print 'Deleted:' (and
the other guys) too. I could argue the other way, too (having these
lines (at all) duplicates the functionality of 'svnlook changed' and is
different from "svn diff"'s output for the same changes), but if we have
some of the tree-change headers I think we should have all of them.
Daniel
(open to be convinced otherwise)
> Best regards,
> Frédéric
>
>
> Daniel Shahaf wrote:
> > Frederic Melot wrote on Fri, 31 Oct 2008 at 17:57 +0100:
> >
> > > Hello,
> > >
> > > you are totally right. My first concern was only on the "Copied:"
> > > headers
> > > for directories and I didn't change anything for "Added:" directories.
> > > I'll
> > > do it.
> > >
> > >
> >
> > Okay. Note that there are a few other variants (e.g. whether the add is
> > of a file or of a dir, whether it is with/without history) -- watch your
> > step, then, don't break one while fixing the other ;)
> >
> >
> > > For the second point, I totally agree. But the code of the
> > > 'print_diff_tree' function is mainly triggerring 3 cases:
> > > 1) the copied files or folders, which update the string buffer and
> > > doesn't
> > > print it.
> > > 2) the files modifications, which update the string buffer and display
> > > it.
> > > It is the only code part in which the string buffer is displayed. This
> > > is
> > > why diffs on folders never appear.
> > > 3) the meta data modifications, a part which does not make use of the
> > >
> >
> > What is "meta data"? Did you mean 'properties'?
> >
> >
> > > string buffer and prints directly the results
> > > As the string buffer was already not fully used I didn't use it. If we
> > > want
> > > to use it more, this function needs to be more deeper modified. Is it
> > > what
> > > you suggested?
> > >
> > >
> >
> > I haven't actually suggested anything yet :)
> >
> > The problem isn't that the stringbuf is underused; it is that its
> > accounting needs some fixes. Currently, sometimes the stringbuf is filled
> > but isn't printed; with your (previous) patch, it was not obvious that
> > there weren't cases where (parts of) the stringbuf would be printed twice.
> >
> > Whether solving the accounting problem should be done by increasing the
> > use of stringbufs I leave for you to decide...
> >
> > Thanks,
> >
> > Daniel
> >
> >
> > > Best regards,
> > > Frederic
> > >
> > > Daniel Shahaf wrote:
> > >
> > > > Frederic Melot wrote on Tue, 21 Oct 2008 at 16:48 +0200:
> > > >
> > > > > Hello,
> > > > >
> > > > > here is a patch submission for issue #3292.
> > > > >
> > > > > Best regards,
> > > > > Frédéric Melot
> > > > >
> > > > >
> > > > Frédéric, thanks for the patch. I have now tried it, and it still
> > > > doesn't print the "Added:" headers for added-without-history
> > > > directories. Could you look into that?
> > > >
> > > > Also, I'm worried that printing the header right away (as you did)
> > > > could
> > > > lead to other problems: for example, how do you ensure that the header
> > > > isn't printed twice (in any case)? That code already isn't the
> > > > easiest
> > > > to follow, and adding prints randomly scattered through won't make it
> > > > better.
> > > >
> > > > Thanks,
> > > >
> > > > Daniel
> > > > ------------------------------------------------------------------------
> > > >
> > > > ---------------------------------------------------------------------
> > > > To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> > > > For additional commands, e-mail: dev-help@subversion.tigris.org
> > > >