Review Request: Review API of the new Path class

[Milian Wolff]

> On Jan. 5, 2013, 11:11 a.m., Andreas Pakulat wrote:
> > project/path.h, line 92
> > <http://git.reviewboard.kde.org/r/108196/diff/1/?file=104951#file104951line92>
> >
> >     Is the string-conversion really needed? I would've hoped that most of the time we're dealing with KUrl's already.
> 
> Milian Wolff wrote:
>     Not if you operate on local files, e.g. Qt uses QString for local paths everywhere. Same for QFileInfo or IndexedString.
> 
> Andreas Pakulat wrote:
>     I guess the latter (IndexedString) is a bit of a problematic case, but for the others I'd say that whoever uses Path needs to do the conversion when it needs to access local files.
>     
>     If it turns out that we end up with tons of this:
>     
>     ... (absfilename is a QString produced here somehow)
>     Path p(KUrl::fromPath(absfilename))
>     
>     Then we can have a convenience constructor, but if thats not needed that often or a KUrl is being used in that place to get a local path for some Qt API and then later on a Path is constructed from the same thing, then I'd leave this constructor out.

Do you want to remove it to make clear that one should use the KUrl ctor if one already has a KUrl? I don't see what you gain by removing this constructor.

Also note that the case you mention (i.e. KUrl::fromPath) should get its own method in Path imo, since in such cases we do not want to go through the costly KUrl and instead split the incoming QString path directly.


- 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/58e52f09/attachment.html>