I'm OK with this one. It's already a long time it's been submitted. Jesper was already alright, so I'll just wait for another OK and apply it. Considering a lot patches from Christophe has already been applied, I hope it won't take long to accept :).

<br><br>Someone else to review it?<br><br>Thanks<br><br><div><span class="gmail_quote">2007/5/18, Christoph Moseler <<a href="mailto:forums@moseler.net">forums@moseler.net</a>>:</span><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">

Hi,<br><br>would it be possible to review and apply the patch?<br><br>Thanks,<br>Christoph<br><br>Christoph Moseler wrote:<br>>  > Will you take care of fixing it?<br>> Sure!<br>> Here's the patch.<br>>

> Cheers, > Christoph > > Jesper K. Pedersen wrote: >> Christoph, I completely agree with you on your diagnostic, and agree >> this is a bug. >> >> Will you take care of fixing it?

<br>>><br>>> Thanks<br>>> Jesper.<br>>><br>>> On Friday 17 November 2006 00:03, Christoph Moseler wrote:<br>>> | Hi Jesper,<br>>> |<br>>> | I'm always wondering why the "Delete select"-dialog behaves a little

<br>>> bit<br>>> | weird when I try to delete files that are read-only. I normally<br>>> | chmod -R a-w<br>>> | the directories containing images (better is better). When I try to<br>>> | delete a read-only image, I would assume that all images that failed to

<br>>> | delete would be kept in the kpa database. Otherwise I would never<br>>> have a<br>>> | chance to correct the file permissions and try the deletion again.<br>>> |<br>>> | Having a look at "void DeleteDialog::deleteImages()" is making clear,

<br>>> | why this happens. I get an error message for these files, but later on<br>>> | the files are deleted from kpa (in DB::ImageDB::instance()->deleteList(<br>>> | _list ) ).<br>>> |<br>>> | Images that failed to be removed from disk should be also removed from

<br>>> | _list so that they are also not removed from the database.<br>>> |<br>>> |<br>>> | BTW: The dialog is doing nothing if neither "Delete images..." nor<br>>> | "Block from..." is checked. In this case, radio buttons would be

>> better. >> | If the user changes his mind at all, he has to press cancel. >> | >> | Well, it's up to you to consider this as an important bug or not, but I >> | think we should fix it sometime.

<br>>> |<br>>> | Cheers,<br>>> | Christoph<br>>> | _______________________________________________<br>>> | KPhotoAlbum mailing list<br>>> | <a href="mailto:KPhotoAlbum-xItUb1CHQy4-XMD5yJDbdMReXY1tMh2IBg@public.gmane.org">

KPhotoAlbum-xItUb1CHQy4-XMD5yJDbdMReXY1tMh2IBg@public.gmane.org</a><br>>> | <a href="http://mail.kdab.net/mailman/listinfo/kphotoalbum">http://mail.kdab.net/mailman/listinfo/kphotoalbum</a><br>>><br>><br>>

<br>> ------------------------------------------------------------------------<br>><br>> Index: MainWindow/DeleteDialog.h<br>> ===================================================================<br>> --- MainWindow/DeleteDialog.h (revision 621690)

<br>> +++ MainWindow/DeleteDialog.h (working copy)<br>> @@ -39,7 +39,6 @@<br>>  private:<br>>      QStringList _list;<br>>      QLabel* _label;<br>> -    QCheckBox* _deleteFromDisk;<br>>      QCheckBox* _block;

<br>><br>>  };<br>> Index: MainWindow/DeleteDialog.cpp<br>> ===================================================================<br>> --- MainWindow/DeleteDialog.cpp       (revision 621690)<br>> +++ MainWindow/DeleteDialog.cpp       (working copy)

<br>> @@ -40,9 +40,6 @@<br>>      _label = new QLabel( top );<br>>      lay1->addWidget( _label );<br>><br>> -    _deleteFromDisk = new QCheckBox( i18n( "Delete images/videos from disk and database" ), top );

<br>> -    lay1->addWidget( _deleteFromDisk );<br>> -<br>>      _block = new QCheckBox( i18n( "Block from database" ), top );<br>>      lay1->addWidget( _block );<br>><br>> @@ -53,7 +50,6 @@

<br>>  {<br>>      _label->setText( i18n("<p><b><center><font size=\"+3\">Delete Images/Videos<br>%1 selected</font></center></b></p>").arg( 

list.count() ) );<br>><br>> -    _deleteFromDisk->setChecked( true );<br>>      _block->setChecked( false );<br>>      _list = list;<br>><br>> @@ -64,26 +60,40 @@<br>>  {<br>>      Utilities::ShowBusyCursor dummy;

<br>><br>> -    if ( _deleteFromDisk->isChecked() ) {<br>> -        for( QStringList::ConstIterator it = _list.begin(); it != _list.end(); ++it ) {<br>> -            Utilities::removeThumbNail( *it );<br>> -            if ( DB::ImageInfo::imageOnDisk(*it) ) {

<br>> -                bool ok = !(QFile( *it ).exists()) ||  QFile( *it ).remove();<br>> -                if ( !ok ) {<br>> -                    KMessageBox::error( this, i18n("Unable to delete file '%1'.").arg(*it),

<br>> -                                        i18n("Error Deleting Files") );<br>> -                }<br>> +    QStringList listToDelete;<br>> +    QStringList listCouldNotDelete;<br>> +<br>> +    for( QStringList::ConstIterator it = _list.begin(); it != _list.end(); ++it ) {

<br>> +        if ( DB::ImageInfo::imageOnDisk(*it) ) {<br>> +            bool ok = !(QFile( *it ).exists()) ||  QFile( *it ).remove();<br>> +            if ( !ok ) {<br>> +                listCouldNotDelete.append

 (*it );<br>> +            } else {<br>> +                listToDelete.append( *it );<br>> +                Utilities::removeThumbNail( *it );<br>>              }<br>>          }<br>>      }<br>><br>> -    if ( _block->isChecked() )

<br>> -        DB::ImageDB::instance()->addToBlockList( _list );<br>> +    if( ! listCouldNotDelete.isEmpty()) {<br>> +        KMessageBox::errorList( this, i18n("<p><b>Unable to delete %1 file(s). Do you have permission to delete these files?</b></p>").arg(

listCouldNotDelete.count()), listCouldNotDelete,<br>> +                            i18n("Error Deleting Files") );<br>> +    }<br>><br>> -    if ( _deleteFromDisk->isChecked() )<br>> -        DB::ImageDB::instance()->deleteList( _list );

<br>> +    if( ! listToDelete.isEmpty()) {<br>> +        if ( _block->isChecked() )<br>> +            DB::ImageDB::instance()->addToBlockList( listToDelete );<br>> +        else<br>> +            DB::ImageDB::instance()->deleteList( listToDelete );

<br>><br>> -    accept();<br>> +        accept();<br>> +<br>> +    } else {<br>> +<br>> +        reject();<br>> +<br>> +    }<br>> +<br>>  }<br>><br>>  #include "DeleteDialog.moc

"<br>><br>><br>> ------------------------------------------------------------------------<br>><br>> _______________________________________________<br>> KPhotoAlbum mailing list<br>> <a href="mailto:KPhotoAlbum-xItUb1CHQy4@public.gmane.org">

KPhotoAlbum-xItUb1CHQy4@public.gmane.org</a><br>> <a href="http://mail.kdab.net/mailman/listinfo/kphotoalbum">http://mail.kdab.net/mailman/listinfo/kphotoalbum</a><br><br>_______________________________________________

<br>KPhotoAlbum mailing list<br><a href="mailto:KPhotoAlbum@kdab.net">KPhotoAlbum@kdab.net</a><br><a href="http://mail.kdab.net/mailman/listinfo/kphotoalbum">http://mail.kdab.net/mailman/listinfo/kphotoalbum</a><br></blockquote>

</div><br><br clear="all"><br>-- <br>Baptiste <Batmat> MATHUS<br>BMathus at Batmat point net - <a href="http://batmat.net">http://batmat.net</a><br>---------<br>Si chacun de nous a une idée et que nous les partageons, nous

repartirons tous les deux avec deux idées... C'est ça le Libre.