mardelle added a comment.
The unittest is in another diff because it's a different author I guess. I
can process the comments but looking closer I found 2 other important issues in
this class:
1. When using KAutoSaveFile::staleFiles(filename) to check if there is an
existing stale file for a document, we use the function
KAutoSaveFile::extractManagedFilePath
This function compares the orginal document path one reconstructed from the
lock file name. However this logic seems flawed since the lock file name uses a
trimmed path in some cases it is impossible to rebuild the original document's
path from it.
Instead I would propose to replace extractManagedFilePath with a new
function, comparing filename, then paths like:
bool staleMatchesManaged(const QString& staleFileName, const QString
managedFile)
{
const QStringRef sep = staleFileName.rightRef(3);
int sepPos = staleFileName.indexOf(sep);
if (QFileInfo(managedFile).fileName() !=
staleFileName.leftRef(sepPos).toString()) {
// File names don't match
return false;
}
int pathPos = staleFileName.indexOf(QChar::fromLatin1('_'), sepPos);
const QByteArray encodedPath = staleFileName.midRef(pathPos+1,
staleFileName.length()-pathPos-1-KAutoSaveFilePrivate::NamePadding).toLatin1();
const QString sourcePathStart = QUrl::fromPercentEncoding(encodedPath);
return
QFileInfo(managedFile).absolutePath().startsWith(sourcePathStart);
}
2. The QLockFile class used in KAutoSaveFile creates a temporary file to
manage locking:
> QLockFile rmlock(d->fileName + QLatin1String(".rmlock"));
https://github.com/qt/qtbase/blob/dev/src/corelib/io/qlockfile.cpp#L259
This means that in our situation where we have a filename that already has
the maximum length, trying to lock will always fail, preventing usage of the
autosave file.
Easier solution is probably to make maximum filename in KAutoSave
File::tempFileName() a bit shorter.
Thoughts? Should I prepare separate patches ? But changing the fileName
encoding as suggested:
> Simpler than toPercentEncoding: use QUrl::fileName(QUrl::FullyEncoded).
Might break more the path recovery method extractManagedFilePath..
REPOSITORY
R244 KCoreAddons
REVISION DETAIL
https://phabricator.kde.org/D24489
To: mardelle, #frameworks, dfaure, mpyne
Cc: ahmadsamir, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
