KUrl to QUrl port

[Milian Wolff]

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
-- 
Milian Wolff
mail at milianw.de
http://milianw.de