[Bug 141786] New: confirmation dialog during rename to existing file does not work

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
         
http://bugs.kde.org/show_bug.cgi?id=141786         
           Summary: confirmation dialog during rename to existing file does
                    not work
           Product: digikam
           Version: unspecified
          Platform: Gentoo Packages
        OS/Version: Linux
            Status: UNCONFIRMED
          Severity: normal
          Priority: NOR
         Component: general
        AssignedTo: digikam-devel kde org
        ReportedBy: torre_cremata mail ru


Version:            (using KDE KDE 3.5.6)
Installed from:    Gentoo Packages

I'd like to rename image file. I press F2 and then enter new file name (image). If such files exist another dialog with appears where I'm asked either skip rename or enter another name.

First problem is that "suggest another file name" button does not work because filename field is empty.
Second problem is that if I type in another file name (say image-1) and click rename I receive an error:

Mailformed URL
image.jpg

In any way, thank you for your time and work on digikam.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Gilles Caulier-2]

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
         
http://bugs.kde.org/show_bug.cgi?id=141786         
caulier.gilles kdemail net changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|general                     |Albums GUI
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
         
http://bugs.kde.org/show_bug.cgi?id=141786         




------- Additional Comments From hugelmopf web de  2007-02-25 00:27 -------
Created an attachment (id=19804)
 --> (http://bugs.kde.org/attachment.cgi?id=19804&action=view)
Patch for renaming images

I am attaching a patch, which fixes this (and followup issues):

1. Problem reported is fixed in dio.cpp line 164. KDE 3.5 API says,
open_RenameDlg wants two complete URLs.

2. With that fixed, renaming works, but updating the album view doesn't,
because ImageInfo still doesn't get the corrected new filename, but the
initially wanted (already existing) filename. So I removed the const qualifier
from the destination URL passed to DIO::renameFile, such that the corrected new
filename can be "returned" to the caller (ImageInfo in this case). Since this
function doesn't seem to be used anywhere else, I suppose, that change won't
introduce problems.

3. There seems to be a rather obvious bug in DIO::renameFile (dio.cpp line
145). We didn't get bitten yet, because we only renamed files within the same
directory, but in case that function is ever called with different source and
target directory, this should be fixed as in my patch.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
         
http://bugs.kde.org/show_bug.cgi?id=141786         




------- Additional Comments From torre_cremata mail ru  2007-02-25 14:52 -------
Thank you for the patch! It fixes reported issue!!!

But there exist another problem. If I press F2 to rename then press Ok on the same name it shows me dialog to rename image. Then I click Replace with the same name ... image disappears from the album until I restart digikam or copy another image in that album.

Well If I replace one image with another I two thumbnails in album but attempt to click them CRASHS digikam.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Marcel Wiesweg]

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
         
http://bugs.kde.org/show_bug.cgi?id=141786         




------- Additional Comments From marcel.wiesweg gmx de  2007-02-25 22:17 -------
Sometimes trying to fix some code shows that it is better removed and rewritten ;-)
So thanks for your patch, but I now think it is better to let KDE libs do the dirty work.
Please test if the commit fixes all the problems.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Marcel Wiesweg]

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
         
http://bugs.kde.org/show_bug.cgi?id=141786         




------- Additional Comments From marcel.wiesweg gmx de  2007-02-25 22:21 -------
SVN commit 637240 by mwiesweg:

Use KIO::rename to rename files
- The AlbumLister would ignore changes to the already listed items.
  Force it to recreate the ImageInfo object by
  a list of invalidated items.
- The thumbnail needs to be invalidated as well.
  This is done by listening to a signal from the KIO::CopyJob
- Remove low-level code from DIO::rename
- Remove actual renaming from ImageInfo::setName

CCBUG: 141786


 M  +25 -7     albumiconview.cpp  
 M  +1 -0      albumiconview.h  
 M  +21 -4     albumlister.cpp  
 M  +6 -0      albumlister.h  
 M  +11 -4     dio.cpp  
 M  +1 -1      dio.h  
 M  +4 -3      imageinfo.cpp  
 M  +2 -5      imageinfo.h  


--- trunk/extragear/graphics/digikam/digikam/albumiconview.cpp #637239:637240
 @ -804,16 +804,34  @
     if (!ok)
         return;
 
-    QString oldURL = item->imageInfo()->kurl().url();
+    KURL oldURL = item->imageInfo()->kurlForKIO();
+    KURL newURL = oldURL;
+    newURL.setFileName(newName + ext);
 
-    if (!item->imageInfo()->setName(newName + ext))
-        return;
+    KIO::CopyJob* job = DIO::rename(oldURL, newURL);
+    connect(job, SIGNAL(result(KIO::Job*)),
+            this, SLOT(slotDIOResult(KIO::Job*)));
+    connect(job, SIGNAL(copyingDone(KIO::Job *, const KURL &, const KURL &, bool, bool)),
+            this, SLOT(slotRenamed(KIO::Job*, const KURL &, const KURL&)));
 
-    d->itemDict.remove(oldURL);
-    d->itemDict.insert(item->imageInfo()->kurl().url(), item);
+    // The AlbumManager KDirWatch will trigger a DIO::scan.
+    // When this is completed, DIO will call AlbumLister::instance()->refresh().
+    // Usually the AlbumLister will ignore changes to already listed items.
+    // So the renamed item need explicitly be invalidated.
+    d->imageLister->invalidateItem(item->imageInfo());
+}
 
-    item->repaint();
-    signalItemsAdded();
+void AlbumIconView::slotRenamed(KIO::Job*, const KURL &, const KURL&newURL)
+{
+    // reconstruct file path from digikamalbums:// URL
+    KURL fileURL;
+    fileURL.setPath(newURL.user());
+    fileURL.addPath(newURL.path());
+
+    // refresh thumbnail
+    d->pixMan->remove(fileURL);
+    // clean LoadingCache as well - be pragmatic, do it here.
+    LoadingCacheInterface::cleanFromCache(fileURL.path());
 }
 
 void AlbumIconView::slotDeleteSelectedItems(bool deletePermanently)
--- trunk/extragear/graphics/digikam/digikam/albumiconview.h #637239:637240
 @ -181,6 +181,7  @
     void slotRemoveTag(int tagID);
 
     void slotDIOResult(KIO::Job* job);
+    void slotRenamed(KIO::Job*, const KURL &, const KURL&);
 
     void slotImageAttributesChanged(Q_LLONG imageId);
     void slotAlbumImagesChanged(int albumId);
--- trunk/extragear/graphics/digikam/digikam/albumlister.cpp #637239:637240
 @ -78,6 +78,7  @
     QString                         filter;
 
     QMap<Q_LLONG, ImageInfo*>       itemMap;
+    QMap<int,int>                   invalidatedItems;
     QMap<int,bool>                  dayFilter;
 
     QValueList<int>                 tagFilter;
 @ -292,6 +293,11  @
     d->filter = nameFilter;
 }
 
+void AlbumLister::invalidateItem(const ImageInfo *item)
+{
+    d->invalidatedItems.insert(item->id(), item->id());
+}
+
 void AlbumLister::slotClear()
 {
     emit signalClear();
 @ -351,6 +357,7  @
     {
         DWarning() << "Failed to list url: " << job->errorString() << endl;
         d->itemMap.clear();
+        d->invalidatedItems.clear();
         return;
     }
 
 @ -365,6 +372,7  @
     }
 
     d->itemMap.clear();
+    d->invalidatedItems.clear();
 
     emit signalCompleted();
 }
 @ -398,13 +406,22  @
         if (d->itemMap.contains(imageID))
         {
             ImageInfo* info = d->itemMap[imageID];
-            if (!matchesFilter(info))
+            d->itemMap.remove(imageID);
+
+            if (d->invalidatedItems.contains(imageID))
             {
+                emit signalDeleteItem(info);
                 emit signalDeleteFilteredItem(info);
+                d->itemList.remove(info);
             }
-            
-            d->itemMap.remove(imageID);
-            continue;
+            else
+            {
+                if (!matchesFilter(info))
+                {
+                    emit signalDeleteFilteredItem(info);
+                }
+                continue;
+            }
         }
 
         ImageInfo* info = new ImageInfo(imageID, albumID, name,
--- trunk/extragear/graphics/digikam/digikam/albumlister.h #637239:637240
 @ -89,6 +89,12  @
     void setDayFilter(const QValueList<int>& days);
     void setTagFilter(const QValueList<int>& tags, const MatchingCondition& matchingCond,
                       bool showUnTagged=false);
+
+    /**
+      * Trigger a recreation of the given ImageInfo object
+      * for the next refresh.
+      */
+    void invalidateItem(const ImageInfo *item);
     
 signals:
 
--- trunk/extragear/graphics/digikam/digikam/dio.cpp #637239:637240
 @ -137,12 +137,18  @
     return job;
 }
 
-bool renameFile(const KURL& src, const KURL& dest)
+KIO::CopyJob *rename(const KURL& src, const KURL& dest)
 {
+    KIO::CopyJob * job = KIO::move(src, dest, false);
+    new Watch(job);
+
+    return job;
+
+    /*
     KURL srcdir;
     srcdir.setDirectory(src.directory());
     KURL dstdir;
-    dstdir.setDirectory(src.directory());
+    dstdir.setDirectory(dest.directory());
     Digikam::PAlbum* srcAlbum = Digikam::AlbumManager::instance()->findPAlbum(srcdir);
     Digikam::PAlbum* dstAlbum = Digikam::AlbumManager::instance()->findPAlbum(dstdir);
     if (!srcAlbum || !dstAlbum)
 @ -161,7 +167,7  @
     while (::stat(QFile::encodeName(dstPath), &stbuf) == 0)
     {
         KIO::RenameDlg_Result result =
-            KIO::open_RenameDlg(i18n("Rename File"), srcPath, KURL(dstPath).fileName(),
+            KIO::open_RenameDlg(i18n("Rename File"), srcPath, dstPath,
                                 KIO::RenameDlg_Mode(KIO::M_SINGLE |
                                                     KIO::M_OVERWRITE),
                                 newDstPath);
 @ -197,7 +203,8  @
 
     KMessageBox::error(0, i18n("Failed to rename file\n%1")
                        .arg(src.fileName()), i18n("Rename Failed"));
-    return false;    
+    return false;
+    */
 }
 
 KIO::Job* scan(const KURL& albumURL)
--- trunk/extragear/graphics/digikam/digikam/dio.h #637239:637240
 @ -38,7 +38,7  @
     
     KIO::Job* del(const KURL::List& srcList, bool useTrash = true);
     
-    bool      renameFile(const KURL& src, const KURL& dest);
+    KIO::CopyJob* rename(const KURL& src, const KURL& dest);
     
     KIO::Job* scan(const KURL& albumURL);
     
--- trunk/extragear/graphics/digikam/digikam/imageinfo.cpp #637239:637240
 @ -86,8 +86,9  @
     return m_name;
 }
 
-bool ImageInfo::setName(const QString& newName)
+void ImageInfo::setName(const QString& newName)
 {
+    /*
     KURL src = kurlForKIO();
     KURL dst = src.upURL();
     dst.addPath(newName);
 @ -101,9 +102,9  @
         DWarning() << "No album found for ID: " << m_albumID << endl;
         return false;
     }
-    
+    */
+
     m_name = newName;
-    return true;
 }
 
 size_t ImageInfo::fileSize() const
--- trunk/extragear/graphics/digikam/digikam/imageinfo.h #637239:637240
 @ -97,13 +97,10  @
     QString   name() const;
 
     /**
-     * Set a new name for the image. This will rename the file on the
-     * disk to the new name. Only use if you are sure of what you are
-     * doing
+     * Set a new name for the image.
      *  param  newName new name for the image
-     *  return true if successful, false otherwise
      */
-    bool      setName(const QString& newName);
+    void setName(const QString& newName);
     
     /**
      *  return the datetime of the image
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
         
http://bugs.kde.org/show_bug.cgi?id=141786         




------- Additional Comments From torre_cremata mail ru  2007-02-26 07:21 -------
Marcel: But is it possible to apply above patch as is or should I recreate patch with svn diff? Currently any attempt to apply the patch gives me:

patch: **** Only garbage was found in the patch input.

And that really does not look like unified patch as it does not contain +++ lines. TIA
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
         
http://bugs.kde.org/show_bug.cgi?id=141786         




------- Additional Comments From hugelmopf web de  2007-02-26 11:45 -------
Marcel: I agree, thanks for fixing it.

Peter: You can undo my patch, and then just update to the latest svn-version, as Marcel has already committed his changes.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Marcel Wiesweg]

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
         
http://bugs.kde.org/show_bug.cgi?id=141786         
marcel.wiesweg gmx de changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |FIXED



------- Additional Comments From marcel.wiesweg gmx de  2007-02-26 21:26 -------
Closing this bug now.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
         
http://bugs.kde.org/show_bug.cgi?id=141786         
torre_cremata mail ru changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |UNCONFIRMED
         Resolution|FIXED                       |



------- Additional Comments From torre_cremata mail ru  2007-03-12 06:38 -------
Installed digikam-0.9.1_rc1 and still have same problems:

1. If I press F2 and then after this just enter (leave the same file name) digikam shows me rename dialog instead of leaving everything as is.

2. Rename dialog does not contain file name inside file name field.

3. Now if I enter already filename which already exist digikam shows me error message: "Mailformed URL" instead of such "filename exist".

4. After previously I've entered already existent filename now if I enter the new filename - I again receive "Mailformed URL" error. (initially reported bug)

This is the same problem, just path to get it became a bit longer.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Gilles Caulier-4]

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
         
http://bugs.kde.org/show_bug.cgi?id=141786         




------- Additional Comments From caulier.gilles gmail com  2007-03-12 07:32 -------
Peter,

Marcel have patched digiKAm source code later 0.9.1-RC1. Try 0.9.1-final release.

Gilles Caulier
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
         
http://bugs.kde.org/show_bug.cgi?id=141786         
torre_cremata mail ru changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |WORKSFORME



------- Additional Comments From torre_cremata mail ru  2007-03-12 08:07 -------
oops. Right.

The last issue that I see is that when renaming leaves the old filename it should not open File already exist dialog. But this is really minor thing.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel