--===============4805479706395323529== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102083/#review7379 ----------------------------------------------------------- directoryusagenotifier/cleanupdirectory.h const KUrl & directoryusagenotifier/cleanupdirectory.cpp Which docu is wrong? I see 0, 1, and 2 being mentionned both in KIO::st= at and in StatJob::setDetails. = OK actually that's for stat(), I don't see any docu for the metadata "d= etails" used by ListJob. Where did you see one? directoryusagenotifier/cleanupdirectory.cpp OUCH, nested event loops are evil, and even more so in kded. Please don= 't use KJob::exec here. Just let the job run. It will autodelete itself, so no need for kill either. directoryusagenotifier/cleanupdirectory.cpp Q_FOREACH would make this simpler and faster (no need to call .at(i) tw= ice) = Also, there's a KFileItem constructor that takes a UDSEntry as input. T= his way it won't have to stat() again every file, to get its attributes and= size. directoryusagenotifier/cleanupdirectory.cpp It would be much faster to sort mEntriesToSort itself, rather than copy= ing into a different container. Just use qSort(begin, end, predicate) and write a predicate function wh= ich sorts two kfileitems the way you want them sorted. directoryusagenotifier/cleanupdirectory.cpp The comment is wrong, this code is sorting by size [use a different pre= dicate function] directoryusagenotifier/cleanupdirectory.cpp ... :) directoryusagenotifier/cleanupdirectory.cpp what's the todo? implementing "deleting the oldest"? (better put the TO= DO inside the method then ;) directoryusagenotifier/cleanupdirectory.cpp Probably due to double encoding. Might be fixed by my suggestion of usi= ng KFileItem(UDSEntry). directoryusagenotifier/cleanupdirectory.cpp Don't use exec. Since these are always local files, I would suggest sim= ply QFile::remove(itemToDelete.localPath()); - David Faure On Sept. 21, 2011, 7:37 p.m., Jaime Torres Amate wrote: > = > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/102083/ > ----------------------------------------------------------- > = > (Updated Sept. 21, 2011, 7:37 p.m.) > = > = > Review request for kdelibs. > = > = > Description > ------- > = > Checks the head of a queue of directories every x minutes (default 30) to= see if this directory (and children) use more space than the allowed (defa= ult 512 Mib). If so, it shows a notification allowing the user to clean the= oldest files, open the file manager, or configure the daemon (time to wait= for the next directory, delete automatically...). > = > = > This addresses bug 79943. > http://bugs.kde.org/show_bug.cgi?id=3D79943 > = > = > Diffs > ----- > = > CMakeLists.txt 1d7c637 = > directoryusagenotifier/CMakeLists.txt PRE-CREATION = > directoryusagenotifier/COPYING PRE-CREATION = > directoryusagenotifier/Messages.sh PRE-CREATION = > directoryusagenotifier/README PRE-CREATION = > directoryusagenotifier/cleanupdirectory.h PRE-CREATION = > directoryusagenotifier/cleanupdirectory.cpp PRE-CREATION = > directoryusagenotifier/directoryusagenotifier.h PRE-CREATION = > directoryusagenotifier/directoryusagenotifier.cpp PRE-CREATION = > directoryusagenotifier/directoryusagenotifier.desktop PRE-CREATION = > directoryusagenotifier/directoryusagenotifier.kcfg PRE-CREATION = > directoryusagenotifier/directoryusagenotifier.notifyrc PRE-CREATION = > directoryusagenotifier/directoryusagenotifier_config.cpp PRE-CREATION = > directoryusagenotifier/directoryusagenotifier_prefs_base.ui PRE-CREATIO= N = > directoryusagenotifier/module.h PRE-CREATION = > directoryusagenotifier/module.cpp PRE-CREATION = > directoryusagenotifier/settings.kcfgc PRE-CREATION = > directoryusagenotifier/tests/CMakeLists.txt PRE-CREATION = > directoryusagenotifier/tests/cleanupunittest.cpp PRE-CREATION = > = > Diff: http://git.reviewboard.kde.org/r/102083/diff/diff > = > = > Testing > ------- > = > It works as expected. > It shows the notification, and deletes the oldest files in the .thumbnail= directory until the files left use less or equal space than the specified. > = > = > Thanks, > = > Jaime Torres Amate > = > --===============4805479706395323529== Content-Type: text/html; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable
This is an automatically generated e-mail. To reply, visit: http://git.revie= wboard.kde.org/r/102083/

= =
directoryusagenotifier/cleanupdirectory.h (Diff revision 4) 

   
private:


  

 

  

    
    

    56
    
    bool rm(KUrl itemToDelete);

  


 





const KUrl &

















 =

 =




 

 
  

   
    directoryusagenotifier/cleanupdirectory.cpp
    

     (Diff revision 4)

    
   
  

 

 
  


   

   
void CleanUpDirectory::setupDirectories()


  

 

  

    
    

    50
    
        // The documentatio=
n sais it should be 1 or 2. But actually it only works with 0.
=

  


 





Which docu is wrong? I see 0, 1, and 2 being mentionned both in KIO:=
:stat and in StatJob::setDetails.

OK actually that's for stat(), I don't see any docu for the metadat=
a "details" used by ListJob. Where did you see one?

















 =

 =




 

 
  

   
    directoryusagenotifier/cleanupdirectory.cpp
    

     (Diff revision 4)

    
   
  

 

 
  


   

   
void CleanUpDirectory::setupDirectories()


  

 

  

    
    

    56
    
        entriesJob->exec(=
);

  


 





OUCH, nested event loops are evil, and even more so in kded. Please =
don't use KJob::exec here. Just let the job run.
It will autodelete itself, so no need for kill either.

















 =

 =




 

 
  

   
    directoryusagenotifier/cleanupdirectory.cpp
    

     (Diff revision 4)

    
   
  

 

 
  


   

   
void CleanUpDirectory::setupDirectories()


  

 

  

    
    

    65
    
        KFileItem file(entry=
List.at(i), KUrl(mCurrent=
URL, entryList.at(=
i).stri=
ngValue(KIO::UDSEntry::UDS_NAME)), true);

  


 





Q_FOREACH would make this simpler and faster (no need to call .at(i)=
 twice)

Also, there's a KFileItem constructor that takes a UDSEntry as input. T=
his way it won't have to stat() again every file, to get its attributes=
 and size.

















 =

 =




 

 
  

   
    directoryusagenotifier/cleanupdirectory.cpp
    

     (Diff revision 4)

    
   
  

 

 
  


   

   
void CleanUpDirectory::setupDirectories()


  

 

  

    
    

    81
    
        sorted.insert(mEntriesToSort.at(i).time(KFileItem::ModificationTime) =
, mEntriesToSort.<=
span class=3D"n">at(i));

  


 





It would be much faster to sort mEntriesToSort itself, rather than c=
opying into a different container.
Just use qSort(begin, end, predicate) and write a predicate function which =
sorts two kfileitems the way you want them sorted.

















 =

 =




 

 
  

   
    directoryusagenotifier/cleanupdirectory.cpp
    

     (Diff revision 4)

    
   
  

 

 
  


   

   
void CleanUpDirectory::setupDirectories()


  

 

  

    
    

    101
    
    // sort the entries by =
ModificationTime

  


 





The comment is wrong, this code is sorting by size [use a different =
predicate function]

















 =

 =




 

 
  

   
    directoryusagenotifier/cleanupdirectory.cpp
    

     (Diff revision 4)

    
   
  

 

 
  


   

   
void CleanUpDirectory::setupDirectories()


  

 

  

    
    

    123
    
    // sort the entries by =
ModificationTime

  


 





... :)

















 =

 =




 

 
  

   
    directoryusagenotifier/cleanupdirectory.cpp
    

     (Diff revision 4)

    
   
  

 

 
  


   

   
void CleanUpDirectory::setupDirectories()


  

 

  

    
    

    143
    
// TODO:

  


 





what's the todo? implementing "deleting the oldest"? (=
better put the TODO inside the method then ;)

















 =

 =




 

 
  

   
    directoryusagenotifier/cleanupdirectory.cpp
    

     (Diff revision 4)

    
   
  

 

 
  


   

   
void CleanUpDirectory::setupDirectories()


  

 

  

    
    

    151
    
    // FIXME: Can not delet=
e files with %20 in the filename

  


 





Probably due to double encoding. Might be fixed by my suggestion of =
using KFileItem(UDSEntry).

















 =

 =




 

 
  

   
    directoryusagenotifier/cleanupdirectory.cpp
    

     (Diff revision 4)

    
   
  

 

 
  


   

   
void CleanUpDirectory::setupDirectories()


  

 

  

    
    

    156
    
        delJob->exec();

  


 





Don't use exec. Since these are always local files, I would sugg=
est simply QFile::remove(itemToDelete.localPath());









- David






On September 21st, 2011, 7:37 p.m., Jaime Torres Amate wrote:









 

  


Review request for kdelibs.


By Jaime Torres Amate.




Updated Sept. 21, 2011, 7:37 p.m.








Descripti=
on 

 

 

  
   
Checks the head of a queue of directories every x minutes (d=
efault 30) to see if this directory (and children) use more space than the =
allowed (default 512 Mib). If so, it shows a notification allowing the user=
 to clean the oldest files, open the file manager, or configure the daemon =
(time to wait for the next directory, delete automatically...).

  		
 






Testing <=
/h1>


 

  
   
It works as expected.
It shows the notification, and deletes the oldest files in the .thumbnail d=
irectory until the files left use less or equal space than the specified.
  

 








 Bugs: 


 79943







Diffs=
 


    

 
  • CMakeLists.txt (1d7c637)
    • 

 
  • directoryusagenotifier/CMakeLists.txt (PRE=
-CREATION)
    • 

 
  • directoryusagenotifier/COPYING (PRE-CREATI=
ON)
    • 

 
  • directoryusagenotifier/Messages.sh (PRE-CR=
EATION)
    • 

 
  • directoryusagenotifier/README (PRE-CREATIO=
N)
    • 

 
  • directoryusagenotifier/cleanupdirectory.h =
(PRE-CREATION)
    • 

 
  • directoryusagenotifier/cleanupdirectory.cpp (PRE-CREATION)
    • 

 
  • directoryusagenotifier/directoryusagenotifier.cpp (PRE-CREATION)
    • 

 
  • directoryusagenotifier/directoryusagenotifier.desktop (PRE-CREATION)
    • 

 
  • directoryusagenotifier/directoryusagenotifier.kcfg (PRE-CREATION)
    • 

 
  • directoryusagenotifier/directoryusagenotifier.notifyrc (PRE-CREATION)
    • 

 
  • directoryusagenotifier/directoryusagenotifier_config.cpp (PRE-CREATION)
    • 

 
  • directoryusagenotifier/directoryusagenotifier_prefs_base.ui (PRE-CREATION)
    • 

 
  • directoryusagenotifier/module.h (PRE-CREAT=
ION)
    • 

 
  • directoryusagenotifier/module.cpp (PRE-CRE=
ATION)
    • 

 
  • directoryusagenotifier/settings.kcfgc (PRE=
-CREATION)
    • 

 
  • directoryusagenotifier/tests/CMakeLists.txt (PRE-CREATION)
    • 





View Diff





  		
 











  

 



--===============4805479706395323529==--