Commit a7466c69 authored by David Redondo's avatar David Redondo 🏎
Browse files

Rework file numbering

Summary:
Previously we looped though every match and checked the filesystem every
iteration. Now we do it only once which should result in better performance. I
also added a test case that shows that the old method was broken, too.

Test Plan: run tests

Reviewers: #spectacle, ngraham

Reviewed By: #spectacle, ngraham

Subscribers: ngraham

Tags: #spectacle

Differential Revision: https://phabricator.kde.org/D28315
parent c871d311
......@@ -198,33 +198,32 @@ QString ExportManager::formatFilename(const QString &nameTemplate)
// check if basename includes %[N]d token for sequential file numbering
QRegularExpression paddingRE;
paddingRE.setPattern(QStringLiteral("%(\\d*)d"));
QRegularExpressionMatch paddingMatch;
while (result.indexOf(paddingRE, 0, &paddingMatch) > -1) {
int highestFileNumber = 0;
// determine padding value
int paddedLength = 1;
if (!paddingMatch.captured(1).isEmpty()) {
paddedLength = paddingMatch.captured(1).toInt();
QRegularExpressionMatchIterator it = paddingRE.globalMatch(result);
if (it.hasNext()) {
QString resultCopy = QRegularExpression::escape(result);
QVector<QRegularExpressionMatch> matches;
while (it.hasNext()) {
QRegularExpressionMatch paddingMatch = it.next();
matches.push_back(paddingMatch);
//determine padding value
int paddedLength = 1;
if (!paddingMatch.captured(1).isEmpty()) {
paddedLength = paddingMatch.captured(1).toInt();
}
QString escapedMatch = QRegularExpression::escape(paddingMatch.captured());
resultCopy.replace(escapedMatch, QLatin1String("(\\d{%1,})").arg(QString::number(paddedLength)));
}
// search save directory for files
QDir dir(baseDir);
const QStringList fileNames = dir.entryList(QDir::Files, QDir::Name);
int highestFileNumber = 0;
// if there are files in the directory...
if (fileNames.length() > 0) {
QString resultCopy = QRegularExpression::escape(result);
QRegularExpression fileNumberRE;
const QString replacement = QStringLiteral("(\\d{").append(QString::number(paddedLength)).append(QLatin1String(",})"));
QString escapedMatch = QRegularExpression::escape(paddingMatch.captured());
const QString fullNameMatch = QStringLiteral("^").append(resultCopy.replace(escapedMatch,replacement)).append(QStringLiteral("\\..*$"));
fileNumberRE.setPattern(fullNameMatch);
fileNumberRE.setPattern(resultCopy);
// ... check the file names for string matching token with padding specified in result
const QStringList filteredFiles = fileNames.filter(fileNumberRE);
// if there are files in the directory that look like the file name with sequential numbering
if (filteredFiles.length() > 0) {
// loop through filtered file names looking for highest number
......@@ -237,8 +236,14 @@ QString ExportManager::formatFilename(const QString &nameTemplate)
}
}
// replace placeholder with next number padded
const QString nextFileNumberPadded = QString::number(highestFileNumber + 1).rightJustified(paddedLength, QLatin1Char('0'));
result.replace(paddingMatch.captured(), nextFileNumberPadded);
for (const auto& match : matches) {
int paddedLength = 1;
if (!match.captured(1).isEmpty()) {
paddedLength = match.captured(1).toInt();
}
const QString nextFileNumberPadded = QString::number(highestFileNumber + 1).rightJustified(paddedLength, QLatin1Char('0'));
result.replace(match.captured(), nextFileNumberPadded);
}
}
// Remove leading and trailing '/'
......
......@@ -67,20 +67,24 @@ void FilenameTest::testWindowTitle()
void FilenameTest::testNumbering()
{
QString lBaseName = QLatin1String("spectacle_test_")+ QUuid::createUuid().toString();
QCOMPARE(mExportManager->formatFilename(lBaseName + QStringLiteral("_%d")), lBaseName + QStringLiteral("_1"));
QCOMPARE(mExportManager->formatFilename(lBaseName + QStringLiteral("_%1d")), lBaseName + QStringLiteral("_1"));
QCOMPARE(mExportManager->formatFilename(lBaseName + QStringLiteral("_%2d")), lBaseName + QStringLiteral("_01"));
QCOMPARE(mExportManager->formatFilename(lBaseName + QStringLiteral("_%3d")), lBaseName + QStringLiteral("_001"));
QCOMPARE(mExportManager->formatFilename(lBaseName + QStringLiteral("_%4d")), lBaseName + QStringLiteral("_0001"));
QCOMPARE(mExportManager->formatFilename(lBaseName + QStringLiteral("_%d_%2d_%3d")),
lBaseName+QStringLiteral("_1_01_001"));
QString BaseName = QLatin1String("spectacle_test_")+ QUuid::createUuid().toString();
QCOMPARE(mExportManager->formatFilename(BaseName + QStringLiteral("_%d")), BaseName + QStringLiteral("_1"));
QCOMPARE(mExportManager->formatFilename(BaseName + QStringLiteral("_%1d")), BaseName + QStringLiteral("_1"));
QCOMPARE(mExportManager->formatFilename(BaseName + QStringLiteral("_%2d")), BaseName + QStringLiteral("_01"));
QCOMPARE(mExportManager->formatFilename(BaseName + QStringLiteral("_%3d")), BaseName + QStringLiteral("_001"));
QCOMPARE(mExportManager->formatFilename(BaseName + QStringLiteral("_%4d")), BaseName + QStringLiteral("_0001"));
QCOMPARE(mExportManager->formatFilename(BaseName + QStringLiteral("_%d_%2d_%3d")),
BaseName+QStringLiteral("_1_01_001"));
QFile lFile(QDir(mExportManager->defaultSaveLocation()).filePath(lBaseName + QStringLiteral("_1.png")));
lFile.open(QIODevice::WriteOnly);
lFile.close();
QCOMPARE(mExportManager->formatFilename(lBaseName+QStringLiteral("_%d")), lBaseName+QStringLiteral("_2"));
lFile.remove();
QFile file(QDir(mExportManager->defaultSaveLocation()).filePath(BaseName + QStringLiteral("_1.png")));
file.open(QIODevice::WriteOnly);
file.close();
QCOMPARE(mExportManager->formatFilename(BaseName+QStringLiteral("_%d")), BaseName+QStringLiteral("_2"));
file.remove();
file.setFileName(QDir(mExportManager->defaultSaveLocation()).filePath(BaseName+QStringLiteral("_1_01_001")));
file.close();
QCOMPARE(mExportManager->formatFilename(BaseName+QStringLiteral("_%d_%2d_%3d")), BaseName+QStringLiteral("_2_02_002"));
file.remove();
}
void FilenameTest::testCombined()
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment