D18161: [WIP/RFC] [kioslave/file] Add a codec for legacy filenames
cfeck added a comment. The encode/decode functions were already reviewed for kdelibs4. It's the remaining code that needs review. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D18161 To: cfeck, #frameworks, #dolphin, dfaure Cc: frispete, nathanshearer, nerdopolist, ngraham, kde-frameworks-devel, michaelh, bruns
D18161: [WIP/RFC] [kioslave/file] Add a codec for legacy filenames
nathanshearer added a comment. Hi Christoph, I just wanted to say thanks for working on this. It will be great when I can interact with these files again via KDE and its suite of apps. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D18161 To: cfeck, #frameworks, #dolphin, dfaure Cc: nathanshearer, nerdopolist, ngraham, kde-frameworks-devel, michaelh, bruns
D18161: [WIP/RFC] [kioslave/file] Add a codec for legacy filenames
dfaure added a comment. Wow, it's nice that this is fixable, I thought it wasn't. I can't really review the code itself though, too clueless about utf8 details. Thiago might be able to review this. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D18161 To: cfeck, #frameworks, #dolphin, dfaure Cc: nerdopolist, ngraham, kde-frameworks-devel, michaelh, bruns
D18161: [WIP/RFC] [kioslave/file] Add a codec for legacy filenames
cfeck added a comment. Thanks for testing, btw. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D18161 To: cfeck, #frameworks, #dolphin, dfaure Cc: nerdopolist, ngraham, kde-frameworks-devel, michaelh, bruns
D18161: [WIP/RFC] [kioslave/file] Add a codec for legacy filenames
cfeck added a comment. It's possible that Kate doesn't use KIO for local files, but falls back to QFile. It _would_ be possible somehow (e.g. via the QPA plugins) to inject the hack into all Qt applications, but I suggest to improve Dolphin in a way that it shows an error message or even a rename dialog when trying to open or execute such a file. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D18161 To: cfeck, #frameworks, #dolphin, dfaure Cc: nerdopolist, ngraham, kde-frameworks-devel, michaelh, bruns
D18161: [WIP/RFC] [kioslave/file] Add a codec for legacy filenames
nerdopolist added a comment. Awesome! For what I can input, base file operations work. I can copy, delete, rename files, and testing, and diffing the trees with my kiocopy test script, it copies them perfectly. However, for example, some of the files I cannot open with kate REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D18161 To: cfeck, #frameworks, #dolphin, dfaure Cc: nerdopolist, ngraham, kde-frameworks-devel, michaelh, bruns
D18161: [WIP/RFC] [kioslave/file] Add a codec for legacy filenames
cfeck updated this revision to Diff 49684. cfeck added a comment. Fix worst case estimate. We need up to two UTF-16 code units for each byte. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D18161?vs=49176&id=49684 REVISION DETAIL https://phabricator.kde.org/D18161 AFFECTED FILES src/ioslaves/file/CMakeLists.txt src/ioslaves/file/file.cpp src/ioslaves/file/legacycodec.cpp src/ioslaves/file/legacycodec.h To: cfeck, #frameworks, #dolphin, dfaure Cc: nerdopolist, ngraham, kde-frameworks-devel, michaelh, bruns
D18161: [WIP/RFC] [kioslave/file] Add a codec for legacy filenames
nerdopolist added a comment. I tried with my script that tried to reproduce that "kio skips files" issue https://raw.githubusercontent.com/n3rdopolis/otherstuff/master/kiocopy (changing the TEST_INVALID_UNICODE_CHARS=0 line to TEST_INVALID_UNICODE_CHARS=1 ) and then letting it run. Unfortunately, I get "The file protocol died unexpectedly REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D18161 To: cfeck, #frameworks, #dolphin, dfaure Cc: nerdopolist, ngraham, kde-frameworks-devel, michaelh, bruns
D18161: [WIP/RFC] [kioslave/file] Add a codec for legacy filenames
ngraham added a reviewer: dfaure. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D18161 To: cfeck, #frameworks, #dolphin, dfaure Cc: ngraham, kde-frameworks-devel, michaelh, bruns
D18161: [WIP/RFC] [kioslave/file] Add a codec for legacy filenames
cfeck created this revision. cfeck added reviewers: Frameworks, Dolphin. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. cfeck requested review of this revision. REVISION SUMMARY **[WIP/RFC] Please let me know if what I propose is sane** UNIX filenames can contain any bytes (except `\0` and `/`). Qt's `QFile::decodeName()` calls `QString::fromLocal8Bit()`, assuming that all filesystems use the system's locale encoding. For filenames that have been created with a different encoding, and have not yet been converted (e.g. using `convmv`), this creates non-reversible U+FFFD (REPLACEMENT CHARACTER) code points in the filenames. For example, some old-style archives might not contain any information about the encoding of the filenames, and even today archivers extract them without trying to convert to the locale's encoding. While full support for those filenames is not needed, Dolphin should at least be able to delete, rename, and move those files. Since all actual (local) file handling is done inside the `file` kioslave, patching Dolphin will not help. This code is a near verbatim copy of the code we had in kdelibs, written by Szókovács Róbert. Only minor adaptions to Qt5 were done. It decodes invalid bytes as U+10FExx from Plane 16 (Supplementary Private Use Area-B) to be able to encode them later. Dolphin could detect filenames with those characters, and either mark them (by color or overlay icon), or even automatically offer to rename them. CCBUG: 204768 CCBUG: 165044 TEST PLAN touch "/tmp/test-"$'\377'".txt" dolphin /tmp Copying and deleting a test file worked with this code, failed without. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D18161 AFFECTED FILES src/ioslaves/file/CMakeLists.txt src/ioslaves/file/file.cpp src/ioslaves/file/legacycodec.cpp src/ioslaves/file/legacycodec.h To: cfeck, #frameworks, #dolphin Cc: kde-frameworks-devel, michaelh, ngraham, bruns
