[KPhotoAlbum] Delete select (images) dialog

[Christoph Moseler]

Hi,

would it be possible to review and apply the patch?

Thanks,
Christoph

Christoph Moseler wrote:
>  > Will you take care of fixing it?
> Sure!
> Here's the patch.
> 
> 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?
>>
>> Thanks
>> Jesper.
>>
>> On Friday 17 November 2006 00:03, Christoph Moseler wrote:
>> | Hi Jesper,
>> |
>> | I'm always wondering why the "Delete select"-dialog behaves a little 
>> bit
>> | weird when I try to delete files that are read-only. I normally
>> | chmod -R a-w
>> | the directories containing images (better is better). When I try to
>> | delete a read-only image, I would assume that all images that failed to
>> | delete would be kept in the kpa database. Otherwise I would never 
>> have a
>> | chance to correct the file permissions and try the deletion again.
>> |
>> | Having a look at "void DeleteDialog::deleteImages()" is making clear,
>> | why this happens. I get an error message for these files, but later on
>> | the files are deleted from kpa (in DB::ImageDB::instance()->deleteList(
>> | _list ) ).
>> |
>> | Images that failed to be removed from disk should be also removed from
>> | _list so that they are also not removed from the database.
>> |
>> |
>> | BTW: The dialog is doing nothing if neither "Delete images..." nor
>> | "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.
>> |
>> | Cheers,
>> | Christoph
>> | _______________________________________________
>> | KPhotoAlbum mailing list
>> | KPhotoAlbum-xItUb1CHQy4-XMD5yJDbdMReXY1tMh2IBg at public.gmane.org
>> | http://mail.kdab.net/mailman/listinfo/kphotoalbum
>>
> 
> 
> ------------------------------------------------------------------------
> 
> Index: MainWindow/DeleteDialog.h
> ===================================================================
> --- MainWindow/DeleteDialog.h	(revision 621690)
> +++ MainWindow/DeleteDialog.h	(working copy)
> @@ -39,7 +39,6 @@
>  private:
>      QStringList _list;
>      QLabel* _label;
> -    QCheckBox* _deleteFromDisk;
>      QCheckBox* _block;
>  
>  };
> Index: MainWindow/DeleteDialog.cpp
> ===================================================================
> --- MainWindow/DeleteDialog.cpp	(revision 621690)
> +++ MainWindow/DeleteDialog.cpp	(working copy)
> @@ -40,9 +40,6 @@
>      _label = new QLabel( top );
>      lay1->addWidget( _label );
>  
> -    _deleteFromDisk = new QCheckBox( i18n( "Delete images/videos from disk and database" ), top );
> -    lay1->addWidget( _deleteFromDisk );
> -
>      _block = new QCheckBox( i18n( "Block from database" ), top );
>      lay1->addWidget( _block );
>  
> @@ -53,7 +50,6 @@
>  {
>      _label->setText( i18n("<p><b><center><font size=\"+3\">Delete Images/Videos<br>%1 selected</font></center></b></p>").arg( list.count() ) );
>  
> -    _deleteFromDisk->setChecked( true );
>      _block->setChecked( false );
>      _list = list;
>  
> @@ -64,26 +60,40 @@
>  {
>      Utilities::ShowBusyCursor dummy;
>  
> -    if ( _deleteFromDisk->isChecked() ) {
> -        for( QStringList::ConstIterator it = _list.begin(); it != _list.end(); ++it ) {
> -            Utilities::removeThumbNail( *it );
> -            if ( DB::ImageInfo::imageOnDisk(*it) ) {
> -                bool ok = !(QFile( *it ).exists()) ||  QFile( *it ).remove();
> -                if ( !ok ) {
> -                    KMessageBox::error( this, i18n("Unable to delete file '%1'.").arg(*it),
> -                                        i18n("Error Deleting Files") );
> -                }
> +    QStringList listToDelete;
> +    QStringList listCouldNotDelete;
> +
> +    for( QStringList::ConstIterator it = _list.begin(); it != _list.end(); ++it ) {
> +        if ( DB::ImageInfo::imageOnDisk(*it) ) {
> +            bool ok = !(QFile( *it ).exists()) ||  QFile( *it ).remove();
> +            if ( !ok ) {
> +                listCouldNotDelete.append (*it );
> +            } else {
> +                listToDelete.append( *it );
> +                Utilities::removeThumbNail( *it );
>              }
>          }
>      }
>  
> -    if ( _block->isChecked() )
> -        DB::ImageDB::instance()->addToBlockList( _list );
> +    if( ! listCouldNotDelete.isEmpty()) {
> +        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, 
> +                            i18n("Error Deleting Files") );
> +    }
>  
> -    if ( _deleteFromDisk->isChecked() )
> -        DB::ImageDB::instance()->deleteList( _list );
> +    if( ! listToDelete.isEmpty()) {
> +        if ( _block->isChecked() )
> +            DB::ImageDB::instance()->addToBlockList( listToDelete );
> +        else
> +            DB::ImageDB::instance()->deleteList( listToDelete );
>  
> -    accept();
> +        accept();
> +
> +    } else {
> +
> +        reject();
> +
> +    }
> +
>  }
>  
>  #include "DeleteDialog.moc"
> 
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> KPhotoAlbum mailing list
> KPhotoAlbum-xItUb1CHQy4 at public.gmane.org
> http://mail.kdab.net/mailman/listinfo/kphotoalbum