[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kfm-devel
Subject:    Re: KIO/KHTML Error Handling Update
From:       David Faure <david () mandrakesoft ! com>
Date:       2002-01-27 13:51:04
[Download RAW message or body]

On Sunday 27 January 2002 14:29, Hamish Rodda wrote:
> BTW did you take a glance at the error:// stuff? was it as expected?

Hmm, I commented on it didn't I?
Ah, crap, looks like the machine crash destroyed that editor before I sent the 
mail.
I was pointing at the fact that
    QStringList urls = KURL::split( url ).toStringList();
    if ( urls.count() == 2 ) {
      QStringList::iterator it2 = urls.end();
      KURL subURL = *(it2++);
involves unnecessary parsing - any QString->KURL conversion means the URL has 
to be parsed. Much more efficient to keep the KURL::List returned by split 
and iterate over it.

More important remark: why use error:// in the sub URL ? My idea was that it 
would be the main URL. This way
* it's faster to check we're in the error:// case, simply 
url.protocol()=="error"
* it also works with URLs that have sub-urls in the first place (the current 
code hardcodes count()==2). It would stack the error:// url on top (i.e. 
first), but after removing it the complete URL can be found again.
* the konq_run patch is missing, right ? ;)

Other than that, it looks good. That's the bugfix part of all this IMHO, so 
I'd be glad if you could finish that part ;)

> That's true, perhaps I should look at that and see if I can come up with a 
> patch. BTW if they are compatible, why can't ioslaves from old builds be 
run? 
> I have a few stale ioslaves (fish for example) that doesn't work.

Compatible with KDE-2 ioslaves linked to KDE-2 libs, not compatible with 
previous kde-3-cvs ones (BIC changes in kde3 kdelibs).

> "Access was denied while trying to {copy, move, delete} the resource". It 
> would be nice to have for about 25% or more of the strings I added.
Hmm, right. Not an easy problem. That's actually more related to the basic 
command (CMD_*) than to the job class (e.g. SimpleJob is about many different 
commands) (and for CopyJob isn't not always about copying: if one can't 
create a dir, it's about a failing mkdir, not about a failing copy, strictly 
speaking). Ok, then maybe an enum and a setJobType() would work, however
e.g. CopyJob (when used for moving) might have to change the job-type from
"Copy" to "Delete" once it enters the "deleting" phase.... Sounds more like 
the "current type of operation" than the actual "job type (constant)".

The classes inheriting from Job are for stuff like handling the result of a 
listRecursive ourselves (e.g. KDirSizeJob or ChmodJob), in which case the job 
type is still mainly about listing, so no problem.

> >> and that the .protocol files
> >> could have a flag added to specify if they are network-based protocols or
> >> not.
> >
> >Yes, looking at your code this looks necessary. You can easily add that,
> >editing all .protocol files, and adding the code to
This sentence lacked the important last word: KProtocolInfo ;)

BTW, glad to see the queuedDetailedError thing. This does need to go in with 
the rest.

-- 
David FAURE, david@mandrakesoft.com, faure@kde.org
http://people.mandrakesoft.com/~david, http://www.konqueror.org
KDE 3.0: Konquering the Desktops
[prev in list] [next in list] [prev in thread] [next in thread]