Second part of fix for a BUG 398998 ( https://bugs.kde.org/show_bug.cgi?id=398998 ).
Kate changes are in D18099.
Details
Details
Printed resulted URL with qDebug().
Diff Detail
Diff Detail
- Repository
- R126 KDE CLI Utilities
- Branch
- cursor
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 7178 Build 7196: arc lint + arc unit
Comment Actions
Overall this makes sense to me, this assumes though that there will never be a file ending with :digit, is that something we can live with?
| kioclient/urlinfo.h | ||
|---|---|---|
| 68 | capturedRef, no need to instantiate the QString to destroy again. | |
Comment Actions
Judging from
UrlInfo(QString path)
: line(0), column(0)
{
/**
* first try: just check if the path is an existing file
*/
if (QFile::exists(path)) {
/**
* create absolute file path, we will e.g. pass this over dbus to other processes
* and then we are done, no cursor can be detected here!
*/
url = QUrl::fromLocalFile(QDir::current().absoluteFilePath(path));
return;
}such files can still be opened, they just can't receive cursor information.
Comment Actions
Is it really necessary to copy urlinfo.h here? Wouldn't be enough to just create a static function and put the URL parsing logic there?
| kioclient/urlinfo.h | ||
|---|---|---|
| 46–53 | Why doesn't it parse the URL if the file already exists? | |
| kioclient/urlinfo.h | ||
|---|---|---|
| 46–53 | If the file is named foo:123, it refuses to search for cursor infomation and just open it. | |
Comment Actions
Is the makeURL function still used, or should it be removed now?
| kioclient/urlinfo.h | ||
|---|---|---|
| 40 | const QString & And what if it's a URL? At this point this string is pathOrUrl. | |
| 78 | I'm not sure about AssumeLocalFile, in the context of kde-open. | |
Comment Actions
Yep, it is still used in a bunch of other places.
| kioclient/urlinfo.h | ||
|---|---|---|
| 40 |
There is path.chop(match.capturedLength());, which requires non-const QString.
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? | |
| 78 | 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". | |
| kioclient/urlinfo.h | ||
|---|---|---|
| 40 | I'm suggesting to at least rename the argument to pathOrUrl to be clear about what it contains. 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). | |
| 78 | 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. | |
| kioclient/urlinfo.h | ||
|---|---|---|
| 78 |
Ah, right. Ok, then. | |
| kioclient/urlinfo.h | ||
|---|---|---|
| 77 | remove old comment | |