Summary: | Special characters when renaming on certain file systems... | ||
---|---|---|---|
Product: | [Applications] juk | Reporter: | Martin Rieder <bugzilla> |
Component: | general | Assignee: | Scott Wheeler <wheeler> |
Status: | REPORTED --- | ||
Severity: | wishlist | CC: | beroot, hartforda, nathan, tom14 |
Priority: | NOR | ||
Version: | 2.0 | ||
Target Milestone: | --- | ||
Platform: | unspecified | ||
OS: | Linux | ||
Latest Commit: | Version Fixed In: | ||
Attachments: | Patch for special charater handling in the file renamer. |
Description
Martin Rieder
2004-03-11 19:54:26 UTC
*** Bug 77553 has been marked as a duplicate of this bug. *** *** Bug 82267 has been marked as a duplicate of this bug. *** Other characters that might cause problems (depending on the file system:) / \ : ? ! & *** Bug 88846 has been marked as a duplicate of this bug. *** For me, on ext3fs Artist: "I Am Kloot" Title: "Untitled #1" REsultant Filename: "I Am Kloot - Untitled" (should be "I Am Kloot - Untitled 1.ogg" or similar). Artist: "The Streets" Title: "Who Got The Funk?" Resultant Filename: "The Streets - Who Got The Funk" (should be "The Streets - Who Got The Funk.mp3"). It fails to rename files with ? in the Album. Jeez... rubbish programmers. First thing I'd think of. Created attachment 7571 [details]
Patch for special charater handling in the file renamer.
Here is a patch for this bug. I have tested it with the string
`¬|!"£$€%^&*()-_=+[]{};:'@#~,<>./?\
on an ext3 partition which worked.
I haven't tested it on vfat, someone else will have to do that. It definately
won't make it any worse than it is already though.
Maybe some commitage? :-) I've actually been on vacation for the last few weeks so I didn't see this before, but I don't think this actually solves the problem at all. It just uses "clean" KURL paths, but those aren't really related at all to the characters that are or aren't allowed on a given file system. It could be that KURL converts them to UTF-8 filenames. Anyhow it is definately better with the patch than without. It certainly behaves more correctly. > It could be that KURL converts them to UTF-8 filenames. Anyhow it is
> definately better with the patch than without. It certainly behaves more
> correctly.
No, sorry -- really it's not at all more correct. It happens to work better in some situations, but that's just a side effect of the way that KURL works and this actually makes the behavior less correct on UNIX file systems.
I don't follow that logic at all. Pros: - Works almost all the time (at least ReiserFS and Ext3fs). - Fails gracefully (either succeeds or fails - no mangled/truncated filenames). - Simpler code. Cons: - *May* be very slightly slower, but this is not a good enough reason not to patch. Can't see any *real* disadvantages. How does it make the behavoir less correct? Is actually working less correct than silently truncating the filename? Can we get a third oppinion? Ah -- now I see what you're getting at. Actually you're fixing a different bug from the one that's reported here, and that's what was confusing me. This wishlist item is actually in reference to certain characters not working on certain file systems -- notably FAT not supporting certain characters and having an option to avoid them. I thought that you were somehow trying to work around that with your patch, which well, couldn't have worked in any sane way. What you actually fixed is a much more recent bug (which didn't exist when this bug was opened) where Frerich mistakenly used the KURL(const QString &) constructor incorrectly. Your patch is more or less functionally equivalent to: - return KIO::NetAccess::file_move(KURL(src), KURL(dest)); + return KIO::NetAccess::file_move(KURL::fromPathOrURL(src), KURL::fromPathOrURL(dest)); But I agree that it's cleaner, so I'll now commit it. Sorry for the mix up -- I was still trying to understand how you could be fixing the original bug with the patch that you gave. CVS commit by wheeler: Patch from Tim Hutt to fix QString -> KURL encoding. This produces similar symptoms to #77315, but actually isn't the same bug. CCBUG:77315 M +34 -26 filerenamer.cpp 1.36 --- kdemultimedia/juk/filerenamer.cpp #1.35:1.36 @@ -20,4 +20,5 @@ #include <kmacroexpander.h> #include <kmessagebox.h> +#include <kstandarddirs.h> #include <qdir.h> @@ -28,4 +29,5 @@ #include <qpainter.h> #include <qsimplerichtext.h> +#include <qstylesheet.h> class FileRenamer::ConfirmationDialog : public KDialogBase @@ -166,14 +168,16 @@ void FileRenamer::rename(PlaylistItem *i return; - if(KMessageBox::warningContinueCancel(0, - i18n("<qt>You are about to rename the file<br/><br/> '%1'<br/><br/> to <br/><br/>'%2'<br/><br/>Are you sure you " - "want to continue?</qt>").arg(item->file().absFilePath()).arg(newFilename), - i18n("Warning"), KStdGuiItem::cont(), "ShowFileRenamerWarning") - == KMessageBox::Continue) { + QString message = i18n("<qt>You are about to rename the file<br/><br/>'%1'<br/><br/> to <br/>" + "<br/>'%2'<br/><br/>Are you sure you want to continue?</qt>"); + message = message.arg(QStyleSheet::escape(item->file().absFilePath())); + message = message.arg(QStyleSheet::escape(newFilename)); + + if(KMessageBox::warningContinueCancel(0, message, i18n("Warning"), KStdGuiItem::cont(), + "ShowFileRenamerWarning") == KMessageBox::Continue) + { if(moveFile(item->file().absFilePath(), newFilename)) item->setFile(FileHandle(newFilename)); else - KMessageBox::error(0, - i18n("<qt>Failed to rename the file<br/><br/>'%1'<br/><br/>to<br/><br/>'%2'</qt>") + KMessageBox::error(0, i18n("<qt>Failed to rename the file<br/><br/>'%1'<br/><br/>to<br/><br/>'%2'</qt>") .arg(item->file().absFilePath()).arg(newFilename)); } @@ -250,23 +254,27 @@ bool FileRenamer::moveFile(const QString return false; - QString dest_ = dest.mid(1); // strip the leading "/" - if(dest_.find("/") > 0) { - const QStringList components = QStringList::split("/", dest_.left( dest.findRev("/"))); - QStringList::ConstIterator it = components.begin(); - QStringList::ConstIterator end = components.end(); - QString processedComponents; - for(; it != end; ++it) { - processedComponents += "/" + *it; - kdDebug(65432) << "Checking path " << processedComponents << endl; - QDir dir(processedComponents); - if(!dir.exists()) { - if (!dir.mkdir(processedComponents, true)) + // Escape URL. + KURL srcURL = KURL::fromPathOrURL(src); + KURL dstURL = KURL::fromPathOrURL(dest); + + // Clean it. + srcURL.cleanPath(); + dstURL.cleanPath(); + + // Make sure it is valid. + if(!srcURL.isValid() || !dstURL.isValid()) return false; - kdDebug(65432) << "Need to create " << processedComponents << endl; - } - } - } - return KIO::NetAccess::file_move(KURL(src), KURL(dest)); + // Get just the directory. + KURL dir = dstURL; + dir.setFileName(QString::null); + + // Create the directory. + if(!KStandardDirs::exists(dir.path())) + if(!KStandardDirs::makeDir(dir.path())) + return false; + + // Move the file. + return KIO::NetAccess::file_move(srcURL, dstURL); } Thanks! :-) In juk 2.1.1, songs with question marks are renamed anyway, and the mp3 tag is stripped off. For example, Less than Jake - Showbiz Science Who Cares.mp3 becomes Less than Jake - Showbiz As of JuK 2.1.2, files names are still trucated at a '?' or a '#', even at directory name level. So that "Oasis/(What's the Story) Morning Glory?/12. Champagne Supernova.mp3" still turns out as "Oasis/(What's the Story) Morning Glory". What's worse, JuK isn't aware that renaming didn't succeed and so it can't find the file and thereby makes one unable to play it and/or modify its tags. It's now correct as of JuK 2.2 (at least as far as question marks are concerned). BTW, slashes in name parts should be somehow quoted (or maybe changed into backslashes or hyphens), as now they end up as path separators, thus breaking nice directory hierarchy. But this is not much of a problem. Yeah slashes should be replaced with %2f - at least thats how KDE does it. Haven't found any reference to this method anywhere on the internet though. And I just noticed that konqueror can't enter strangly named directories. Eg, make a directory structure like this and the try and browse it in konqueror - doesn't work! ~/Test/Album/Hypetraxx - !"£$%^&*()/?-={}_+#'/;~@:???123 |