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

List:       kde-core-devel
Subject:    Re: Filelight into kdereview (again)
From:       Martin Sandsmark <sandsmark () samfundet ! no>
Date:       2010-09-25 12:45:45
Message-ID: 201009251445.46331.sandsmark () samfundet ! no
[Download RAW message or body]

On Monday 20. September 2010 21.09.11 Friedrich W. H. Kossebau wrote:
> 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.

I wouldn't call it strict, rather thorough (which I think is a good thing). 
:-)


> * 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!

I can't reproduce this here. If you start the stand-alone application, does it 
still display redundant information about partitions?
I think someone mentioned on IRC that this was due to some Solid backends 
reporting some partitions several times or something. I have now added a 
simple workaround in Filelight, though (skipping already added partitions).


> * embedded in Konqueror, the Settings menu contains two times the entry
> "Configure Konqueror...", while one refers to the settings of the KPart

I guess this is a kdelibs bug? I just use KStandardAction::preferences. I'll 
see if I have time later to try to track it down.


> * maybe this is due to current kdelibs (mine is from some weeks ago):
> filelight(4795) Filelight::LocalLister::readMounts: Found the following
> local filesystems:  ("/boot")

I'm sorry, I might be a bit dense now (just woke up), but what is wrong here? 
:-)


> * 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...

It was attempting to paint on a null pixmap. Fixed now.


> * embedded in Konqueror, the KPart seems to not integrate it's statusbar
> with the one of the Konqueror window which looks strange.

Hmm, I seem to be getting a three-line high status bar, or close to, at least. 
I can't see anything blatantly wrong I'm doing with the KParts API, though. I 
can try to dig into konqueror to see why this is happening later.


> * 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 ;) )

I'll write it up on my todolist.


> * 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.

I'm not sure I understand you completely. You mean to keep the last selection 
for a small amount of time when Filelight loses focus?


> Code-wise:
> ----------
> * LocalLister::readMounts()
>     remoteFsTypes << QLatin1String( "smbfs" ) << QLatin1String( "nfs" ) <<
> QLatin1String( "afs" ); //TODO: expand
> -> perhaps better a whitelist than a blacklist?

There are (as far as I know) more diversity in local filesystems than in 
remote ones, hence why I chose to continue with the shortest list. The best 
thing would be to have this information from solid, though.


> * MyRadialMap::setCursor(...)
> focusSegment()->file()->name() == QLatin1String( "Used" )
> -> will fail for a file named "Used", or?

This is just used for the summary widget (when you open Filelight), and we 
abuse the radial map widget a bit to display the available partitions. :-)


> * 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

I was too lazy (*cough* busy) to write up a test case for them at the time, 
but it seems like it isn't happening anymore. But I'm not sure when it went 
away, though, so I'll keep the checks in place.


> 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).

I think this is it. :-)


> 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 :)

I think it should be sane now.

-- 
Martin T. Sandsmark
[prev in list] [next in list] [prev in thread] [next in thread]