Filelight into kdereview (again)

[Friedrich W. H. Kossebau]

Hi Martin,

current coordinator of kdeutils module now replying :)

Dimanche, le 12 septembre 2010, à 21:07, Martin Sandsmark a écrit:
> Hi!
> 
> I'm going to attempt this again; moving Filelight into kdeutils (last time
> I unfortunately didn't have spare time to fix all the outstanding issues).
> Now I'm mostly finished with the volunteer work at the student society,
> and the early-semester buzz at the uni has quieted down, and I hope to
> have more time to spend working on fun things. :-)
> 
> Thanks to Friedrich for the last review, and also for fixing a lot of the
> issues he raised. :-)
> (Also thanks to everyone else who has helped out.)
> 
> AFAICS, the only unsolved issue left (from Friedrich's last review) is the
> feature request for a more complete tooltip for files and folders, but I
> don't think it's really a vital feature to have (though I have written it
> up on my todo-list).
> 
> So if the good people of KDE would be so kind as to look over Filelight and
> the documentation, I would be most pleased.

Good to have learned meanwhile that the strict review last time was not too 
discouraging :) And even better to see you offering the fruits of your (and 
ancestors') labour again for general sharing as part of kdeutils.

So let's see again :)

User experience:
---------------
* embedded in Konqueror, if you select the root path, so that scanning takes 
some time, then press the stop button, the scanning stops now (yeah :)), but 
leaves me with two(!) circles side-by-side, both seeming to show the info 
about the /boot folder. FIXME!
* embedded in Konqueror, the Settings menu contains two times the entry 
"Configure Konqueror...", while one refers to the settings of the KPart
* maybe this is due to current kdelibs (mine is from some weeks ago): 
filelight(4795) Filelight::LocalLister::readMounts: Found the following local 
filesystems:  ("/boot")
* this is not, might be rather still problem in code:
QPainter::begin: Paint device returned engine == 0, type: 2
filelight(4795) RadialMap::Map::paint: Failed to initialize painting, 
returning... 
* embedded in Konqueror, the KPart seems to not integrate it's statusbar with 
the one of the Konqueror window which looks strange.
* there could be a hint in a tooltip on the center circle that clicking this 
area will move the view up one level in the hierarchy (only discovered it by 
hard testing ;) )
* there could be a small delay until a new section is selected on mouse-over 
hovering, similar like there is with entries in menus. sometimes one just 
wants to move the mouse cursor out of view or to do something in another 
window and still keep the current selection like it is.

Code-wise:
----------
* LocalLister::readMounts()
    remoteFsTypes << QLatin1String( "smbfs" ) << QLatin1String( "nfs" ) << 
QLatin1String( "afs" ); //TODO: expand
-> perhaps better a whitelist than a blacklist?
* MyRadialMap::setCursor(...)
focusSegment()->file()->name() == QLatin1String( "Used" )
-> will fail for a file named "Used", or?
* DiskList::DiskList()
    foreach (const Solid::Device &device, 
Solid::Device::listFromType(Solid::DeviceInterface::StorageAccess))
    { // Iterate over all the partitions available.

        if (!device.is<Solid::StorageAccess>()) continue; // It happens.
-> reported to Solid guys? Should not happen! seen this also in another place
* too lazy to search for more items to complain :P

Oh dear, just listed the things I did not like, forgot to list those I do :) 
All in all looks alright, working and usable, think it should/could finally 
move over into kdeutils, once at least the one item above marked with FIXME is 
fixed (could be just my broken trunk, though).

Summary:
Hoping you will by the time iron out the other stuff I mentioned, my thumb is 
up to have filelight move into kdeutils, once the behaviour on pressing the 
stop button during a scanning process is assured to be sane :)

Cheers
Friedrich
-- 
KDE Okteta - a simple hex editor - http://utils.kde.org/projects/okteta