Review Request: Make folderview follow some guidelines

[Aaron Seigo]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.vidsolbach.de/r/307/#review292
-----------------------------------------------------------


very close .. just a couple comments and then this should go in =)


/trunk/KDE/kdebase/apps/plasma/applets/folderview/folderview.cpp
<http://reviewboard.vidsolbach.de/r/307/#comment242>

    path = QDir::homePath()?
    
    it's needed in both branches of the if statement, lets you get rid of the !isContainment() branch and ensures that path is never isEmpty(), which right now it can be if isContainment() && (desktopPath == QDir::homePath() || !desktopFolder.exists())



/trunk/KDE/kdebase/apps/plasma/applets/folderview/folderview.cpp
<http://reviewboard.vidsolbach.de/r/307/#comment241>

    should be "//FIXME: 4.3" so we don't lose it =)



/trunk/KDE/kdebase/apps/plasma/applets/folderview/folderview.cpp
<http://reviewboard.vidsolbach.de/r/307/#comment243>

    this should just be removed, no? the warning should now only show if it's been configured to a non-existent directory, which should never be the case by default with your patch ...


- Aaron


On 2008-12-13 14:58:53, Artur de Souza wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.vidsolbach.de/r/307/
> -----------------------------------------------------------
> 
> (Updated 2008-12-13 14:58:53)
> 
> 
> Review request for Plasma, Anne-Marie Mahfouf and Aaron Seigo.
> 
> 
> Summary
> -------
> 
> This follows aseigo's ideas:
> 
> a) if it's user placed plasmoid, it should always default to $HOME
> 
> b) the one we create automatically at the start in the plasma desktop shell defaults to $DESKTOP if it exists (and isn't $HOME) as a compromise position --> (this one is done by desktopcorona.cpp)
> 
> c) when it's a containment, it should default to $DESKTOP if it exists (and isn't $HOME) since that is a legacy compatibility mode
> d) in no circumstances should plasma trigger the creation of a $DESKTOP folder path if it doesn't point to something valid
> 
> 
> We still have to change the description of folderview when we unfreeze the strings, making bug 175824 invalid.
> 
> 
> This addresses bugs 175824 and 176652.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdebase/apps/plasma/applets/folderview/folderview.cpp
> 
> Diff: http://reviewboard.vidsolbach.de/r/307/diff
> 
> 
> Testing
> -------
> 
> Tested with trunk in different situations. Important to say that we have a new bug: if the user changes the desktop path in "Settings>>About me", the folderview pointing to desktop:/ is not refreshed.
> 
> 
> Thanks,
> 
> Artur
> 
>