Re: [TSVN] Tentative path for SVNFolderStatus

At 15:19 28/10/2004 +0200, you wrote:

>The check for "not a folder" shoudln't be necessary, because we never
>should get here if it's a folder. I know, the if
>(shellCache.IsRecursive() && !PathFileExists(exfile)) makes you think
>otherwise (and I agree, we just should return svn_wc_status_normal if
>that expression is false) but the 'parent' function which acutally
>calls this one already filters out folders when in non-recursive mode.

You might think that, but it doesn't. My observations are not based on a
dry code review, but on annotating the code and running it.

Unfortunately, I posted a long message about this earlier this morning
which appears to have gone missing - I'll resend it.

>So to fix that, I suggest to just
>return &invalidstatus;

OK, I'll look at that.

> >
> > - internalpath = svn_path_internal_style
> > (CUnicodeUtils::StdGetUTF8(pathbuf).c_str(), pool);
> > + if(isFolder)
> > + {
> > + internalpath = "";
> > + }
> > + else
> > + {
> > + internalpath = svn_path_internal_style
> > (CUnicodeUtils::StdGetUTF8(pathbuf).c_str(), pool);
> > + }
> > ctx->auth_baton = NULL;
>
>Yikes!
>Remember: a few lines above, you remove the filename part from the
>path. That means here, it always _is_ a folder, no matter what
>isFolder tells you!

OK, good point. In which case we should always do the "" evaluation.

It wouldn't give an error, just a loss of an optimisation. In fact, what
happens in real life is that we come here first with a folder, pick up
*all* its files in the first pass, and then the cache catches all the file
status requests.

>Don't do that here. It's already done at the beginning of the COM-function.

OK.

>Drop the if (isFolder) and you're fine.

OK.

>Why do you want to remove that?

Because of the _fullpath lower down, which fills the cache with paths with
\ instead of /.

(As an aside, doing the strlen in the for() loop is rather inefficient!)

Even if a transform of the filename was required, I'd suggest it's better
to do it at the point things go into the cache, rather than the point they
come out.

> + }

>AFAIK Subversion returns full paths here, no matter what path you pass
>in for the function. Don't know for sure, I have to test that first.

It doesn't. It returns short paths if you ask it to do the current directory.
That's why I put the _fullpath in.

Cheers,

Will

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tortoisesvn.tigris.org
For additional commands, e-mail: dev-help@tortoisesvn.tigris.org
Received on Thu Oct 28 16:37:48 2004