Review Request: Review API of the new Path class

Milian Wolff mail at milianw.de
Sun Jan 6 16:49:51 UTC 2013



> On Jan. 5, 2013, 11:11 a.m., Andreas Pakulat wrote:
> > project/path.h, line 193
> > <http://git.reviewboard.kde.org/r/108196/diff/1/?file=104951#file104951line193>
> >
> >     Is this really needed? I don't like transforming a Path to a String due to loosing all type information. I think we should only allow accessing individual path segments (like a list) or getting a fully-qualified KUrl out of the clas (and into it).
> 
> Milian Wolff wrote:
>     For local files this is much faster than a KUrl. And that is neat for checks in e.g. QFile::exists or for setting the text in a label or such.
>     
>     You sould take a look at the benchmarks, KUrl is just dog slow.
> 
> Andreas Pakulat wrote:
>     We could add such convenience API to Path itself if its used in many places, i.e. Path::exists(), Path::isDirectory(), Path::isFilename() (does a Path ever denote a relative path? Can't see how to construct that at the moment).
>     
>     If you stick to a QString-getter, I'd probably go for a single one for now, i.e. toLocalFile and drop path and pathOrUrl. Then see how many places would actually not care about wether the Path is for a real url or a local file but need a QString and then decide to re-add whats needed.

I dislike dropping pathOrUrl in favor of toLocalFile since the latter has the added semantics of returning an empty string for remote paths.

::path() otoh can be dropped, I concur.


- Milian


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/108196/#review24736
-----------------------------------------------------------


On Jan. 5, 2013, 10:59 a.m., Andreas Pakulat wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108196/
> -----------------------------------------------------------
> 
> (Updated Jan. 5, 2013, 10:59 a.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Description
> -------
> 
> Discuss API of the new Path class
> 
> 
> Diffs
> -----
> 
>   project/path.h PRE-CREATION 
>   project/path.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/108196/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andreas Pakulat
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20130106/f6bc2cef/attachment.html>


More information about the KDevelop-devel mailing list