KUrl to QUrl port

[Aleix Pol]

On Sun, Sep 14, 2014 at 12:26 AM, Milian Wolff <mail at milianw.de> wrote:

> On Saturday 13 September 2014 16:29:20 Milian Wolff wrote:
> > Hello all!
> >
> > At Akademy, I spent the last 2 days or so porting our code from KUrl to
> > QUrl. This is pretty tricky as we use URLs in (too) many places of our
> API.
> > Locally, I have it now compiling and it even seems to work OK. I'll
> rebase
> > it on the recent changes today and then push it. Some notes below, please
> > read.
> >
> > NOTE: I won't have time to port all external plugins! Please take care of
> > this if you maintain a plugin. If you need help, please ask here on the
> > list.
> >
> > Porting notes:
> >
> > The most important thing is to make the QUrl <=> QString() conversion
> > explicit. That can be done via
> > add_definitions(-DQT_NO_URL_CAST_FROM_STRING).
> >
> > Second step then is to use kde/kdesdk/kde-dev-scripts/kf5/
> convert-kurl.pl on
> > your files. This will do the most tedious porting work for you. But it's
> by
> > far not enough. You'll have to review all changes.
> >
> > In general, if you do not have to interact with QUrl API, prefer
> > KDevelop::Path if possible. This is trivial to port to, as it mimicks the
> > KUrl API nearly 1-to-1.
> >
> > If you are only interested in local files, QDir and QFileInfo are also
> good
> > options to use.
> >
> > When you do need QUrl's, then here are some remarks:
> >
> > Never do QUrl(IndexedString(...).str()). Always use
> IndexedString.toUrl().
> >
> > Nearly use QUrl(QString). Only do this if you want to create a relative
> URL
> > which you then pass to QUrl::resolved or similar. Otherwise e.g. a
> > QUrl("/foo/bar") would create a relative URL, not an absolute local file
> > URL. I've added checks for that in many central places, so if you do
> > something wrong, you'll hopefully get an assertion. In general, prefer
> > QUrl::fromLocalFile where appropriate, or QUrl::fromUserInput otherwise.
> >
> > Never use QUrl::resolved with a ".." to emulate KUrl::cdUp. Either use
> > KIO::upUrl or use QUrl::adjusted(QUrl::RemoveFilename |
> > QUrl::StripTrailingSlash).
> >
> > During the port I noticed that we abuse QUrl in far too many places of
> our
> > API. A long-term goal will be to refactor these places. The VCS subsystem
> > e.g. uses QUrl to represent file paths, which is overkill and easy to get
> > wrong (see above). KDevelop::Path would be a better choice here. In
> > contrast, we'll need to refactor the WorkingSets code to use QUrl instead
> > of QString for the "specifier" which gets stored there a lot. Finally, I
> > really want to see an IndexedPath in our DUChain code to make that code
> > more typesafe and explicit.
>
> Hey all,
>
> I've now pushed my mega porting patch. I've also fixed quite a few more
> things
> while at it, and potentially broke hell loose. So please test this and
> check
> whether things are working as expected. Note that I added a lod of paranoid
> assertions in our codebase. These might need to be re-evaluated and/or
> removed
> over the next period. They helped me find wrong usage of QUrl though, so I
> think we should try to get our code working with it.
>
> There are still some unit tests failing for me, most notably in CMake and
> old-
> Cpp. I think most where failing even without my patches, but maybe I
> introduced new issues as well.
>
> I hope that together we can sort this all out.
>
> Bye
>

There's only a test not passing now on kdevelop:
http://build.kde.org/job/kdevplatform_master_qt5/
http://build.kde.org/job/kdevelop_master_qt5/

Amazing "GNU C Compiler Warnings Trend" plot ;)

Aleix
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20140915/3a257cb1/attachment-0001.html>