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.
Created attachment 19804 [details] 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.
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.
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.
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
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
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.
Closing this bug now.
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.
Peter, Marcel have patched digiKAm source code later 0.9.1-RC1. Try 0.9.1-final release. Gilles Caulier
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.