D18296: Add support for passing cursor information via URL parameters when running kioclient exec.

David Faure noreply at phabricator.kde.org
Tue Apr 9 23:14:37 BST 2019


dfaure added inline comments.

INLINE COMMENTS

> arrowd wrote in urlinfo.h:39
> > const QString &
> 
> There is `path.chop(match.capturedLength());`, which requires non-const `QString`.
> 
> > And what if it's a URL? At this point this string is pathOrUrl.
> 
> Well, `if (QFile::exists(path))` will return false in this case, and `url` would get populated by `url = QUrl::fromUserInput()`. What's wrong with that?

I'm suggesting to at least rename the argument to pathOrUrl to be clear about what it contains.
A path is not a URL.

I see, about the `chop()`. It is customary, however, to make a local copy where needed. For instance this will avoid the copy when the file exists (and we get to the early return, before the copy that you'll need only further down).

> arrowd wrote in urlinfo.h:77
> It is not about creating missing files, but reaction to user typos. If I try to open `fiel.txt` instead of a `file.txt`, I want to get a "no such file or directory error message" instead of popping browser trying to open "http://fiel.txt".

But then you can't do `kde-open5 www.google.fr` anymore, right?

I see what you mean with typo handling, but there is no perfect solution. Either we treat typos as URLs (but it means we also treat actual URLs as such), or we treat everything non-existing as a local file (breaking any use of short URLs). The latter is OK for kwrite, but not for the more general purpose kioclient / kde-open5.

REPOSITORY
  R126 KDE CLI Utilities

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

To: arrowd, #plasma, #ktexteditor, broulik, #frameworks, pino, cfeck, dfaure, elvisangelaccio
Cc: apol, cullmann, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20190409/bdd04885/attachment-0001.html>


More information about the Plasma-devel mailing list