dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.


  For SMB shares I guess what can happen is that the URL doesn't have a 
username in it, but it still needs a password? IIRC there was no username in 
"old" samba (W95 <https://phabricator.kde.org/W95>, W98 
<https://phabricator.kde.org/W98>, XP, ...?).
  This might explain what you were saying about people having password problems 
even with empty userInfo() (which means username and password).
  But we can hardly know, just by looking at a URL, if a password is needed... 
I guess most of time a password is actually needed? Maybe you're right after 
all, and we should always send SMB urls to kiofuse..... Dunno.

INLINE COMMENTS

> desktopexecparser.cpp:329
> +    } else if (!appSupportedProtocols.contains(QLatin1String("KIO"))) {
> +        for (int i = 0; i < d->urls.length(); ++i)  {
> +            const QUrl url = d->urls.at(i);

s/length/count/

> desktopexecparser.cpp:337
> +            // @see https://bugs.kde.org/show_bug.cgi?id=330192
> +            if(!supported || (!url.userName().isEmpty() && 
> url.password().isEmpty())) {
> +                requests.push_back({ kiofuse_iface.mountUrl(url.toString()), 
> i });

coding style: missing space after "if"

> desktopexecparser.cpp:374
> +             d->urls[request.urlIndex] = 
> QUrl::fromLocalFile(request.reply.value());
> +     }
> +

indentation of those last 5 lines seems wrong (2 spaces instead of 4?)

> feverfew wrote in desktopexecparser.cpp:323
> One can always remove/uninstall kiofuse if it doesn't work or if they don't 
> really want the feature?

OK, that's a possible solution indeed.

> feverfew wrote in desktopexecparser.cpp:356
> In these methods I have to return the URLs within the function, so blocking 
> is inevitable. If I could return the URLs via a signal then blocking could be 
> avoided.

I'll try to keep this in mind when coming back to my KJob-based rewrite of KRun.
I keep postponing it because people keep changing KRun :-)

REPOSITORY
  R241 KIO

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

To: feverfew, fvogt, davidedmundson, dfaure, ngraham
Cc: alexde, broulik, sitter, davidedmundson, kde-frameworks-devel, ngraham, 
LeGast00n, GB_2, michaelh, bruns

Reply via email to