D29385: Introduce KIO::OpenUrlJob, a rewrite and replacement for KRun

[David Faure]

dfaure marked 2 inline comments as done.
dfaure added inline comments.

INLINE COMMENTS

> broulik wrote in openurljob.cpp:261
> I know what you always say when I say what I always say, but why not just always stat/mimetype job?

LOL we're like an old couple, the explicit discussion doesn't actually need to happen anymore ;)

OK for everyone else, we're debating whether it's ok to use blocking local-file I/O like QFile or QMimeDatabase which reads from the file.

Of course less code paths is a good thing for maintenance, but it seems *so* overkill to make two calls to a kioslave just to find out the mimetype of a file.... My main counter argument is performance.

For the whole OpenUrlJob until the mimetype is found:

With KIO

  RESULT : OpenUrlJobTest::takeOverAfterMimeTypeFound():
   0.29 msecs per iteration (total: 75, iterations: 256)

With the local-file optimization

  RESULT : OpenUrlJobTest::takeOverAfterMimeTypeFound():
   0.0986 msecs per iteration (total: 101, iterations: 1024)

That's 3 times faster. Admittedly this is not the kind of things people do in a loop.

Well, OK, if nobody objects I can remove the local-files fast paths.
0.2ms is nothing when lharming 2018 PG Demi Lovato Ashley Tisdale Avril Lavigne-02052020.mpgaunching an app, or even when opening a URL in a browser.

[More context: QMimeDatabase *might* determine the mimetype from just the extension, in which case no I/O happens and we could do that here, or it might need to look into the contents of the file. We can ask it to not do that but then the mimetype determination will be less good, for some mimetypes; and we can't ask it if we would get better information by looking at content, so there's no way to split up the work between here and the kioslave. It's "quick search" vs "full search", not phase 1, phase 2.]

> broulik wrote in openurljob.cpp:447
> was or should be?

Hehe, see kdesktopfileactions.cpp:141 ;-)

> broulik wrote in openurljob.cpp:506
> While at it, can we clean up/unify those texts? Sometimes it puts the file on a new line, sometimes it's bold, sometines in quotes, etc. Generally I wouldn't really want any HTML formatting in there.

Completely agree. I removed most of the HTML formatting when grabbing the error messages from KRun, looks like I forgot this one.

So now it's quotes everywhere.

But indeed sometimes the filename is on its own line. Should I just use quotes everywhere?

I guess the problem is when the filename (with full path) is very long.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D29385

To: dfaure, ahmadsamir, broulik, meven, kossebau, davidedmundson, nicolasfella, svuorela
Cc: feverfew, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200503/f0139731/attachment-0001.html>