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
