Organize: Don't allow organizing files without unique tags

Fixes #1077
This commit is contained in:
Jonas Kvinge 2022-11-26 23:37:41 +01:00
parent 2cb29d06b3
commit 7c0c9fccdb
6 changed files with 159 additions and 129 deletions

View File

@ -55,9 +55,11 @@ class Organize : public QObject {
public:
struct NewSongInfo {
explicit NewSongInfo(const Song &song = Song(), const QString &new_filename = QString()) : song_(song), new_filename_(new_filename) {}
explicit NewSongInfo() : unique_filename_(false) {}
explicit NewSongInfo(const Song &song, const QString &new_filename, const bool unique_filename) : song_(song), new_filename_(new_filename), unique_filename_(unique_filename) {}
Song song_;
QString new_filename_;
bool unique_filename_;
};
using NewSongInfoList = QList<NewSongInfo>;

View File

@ -72,8 +72,8 @@
# include "transcoder/transcoder.h"
#endif
const char *OrganizeDialog::kSettingsGroup = "OrganizeDialog";
const char *OrganizeDialog::kDefaultFormat = "%albumartist/%album{ (Disc %disc)}/{%track - }{%albumartist - }%album{ (Disc %disc)} - %title.%extension";
constexpr char OrganizeDialog::kSettingsGroup[] = "OrganizeDialog";
constexpr char OrganizeDialog::kDefaultFormat[] = "%albumartist/%album{ (Disc %disc)}/{%track - }{%albumartist - }%album{ (Disc %disc)} - %title.%extension";
OrganizeDialog::OrganizeDialog(TaskManager *task_manager, CollectionBackend *backend, QWidget *parentwindow, QWidget *parent)
: QDialog(parent),
@ -456,13 +456,20 @@ Organize::NewSongInfoList OrganizeDialog::ComputeNewSongsFilenames(const SongLis
Organize::NewSongInfoList new_songs_info;
new_songs_info.reserve(songs.count());
for (const Song &song : songs) {
QString new_filename = format.GetFilenameForSong(song, extension);
if (filenames.contains(new_filename)) {
QString song_number = QString::number(++filenames[new_filename]);
new_filename = Utilities::PathWithoutFilenameExtension(new_filename) + "(" + song_number + ")." + QFileInfo(new_filename).suffix();
OrganizeFormat::GetFilenameForSongResult result = format.GetFilenameForSong(song, extension);
if (result.filename.isEmpty()) {
return Organize::NewSongInfoList();
}
filenames.insert(new_filename, 1);
new_songs_info << Organize::NewSongInfo(song, new_filename);
if (result.unique_filename) {
if (filenames.contains(result.filename)) {
QString song_number = QString::number(++filenames[result.filename]);
result.filename = Utilities::PathWithoutFilenameExtension(result.filename) + "(" + song_number + ")." + QFileInfo(result.filename).suffix();
}
else {
filenames.insert(result.filename, 1);
}
}
new_songs_info << Organize::NewSongInfo(song, result.filename, result.unique_filename);
}
return new_songs_info;
@ -512,18 +519,23 @@ void OrganizeDialog::UpdatePreviews() {
bool ok = format_valid && !songs_.isEmpty();
if (capacity != 0 && total_size_ > free) ok = false;
ui_->button_box->button(QDialogButtonBox::Ok)->setEnabled(ok);
if (!format_valid) return;
QString extension;
if (ok) {
QString extension;
#ifdef HAVE_GSTREAMER
if (storage && storage->GetTranscodeMode() == MusicStorage::Transcode_Always) {
const Song::FileType format = storage->GetTranscodeFormat();
TranscoderPreset preset = Transcoder::PresetForFileType(format);
extension = preset.extension_;
}
if (storage && storage->GetTranscodeMode() == MusicStorage::Transcode_Always) {
const Song::FileType format = storage->GetTranscodeFormat();
TranscoderPreset preset = Transcoder::PresetForFileType(format);
extension = preset.extension_;
}
#endif
new_songs_info_ = ComputeNewSongsFilenames(songs_, format_, extension);
new_songs_info_ = ComputeNewSongsFilenames(songs_, format_, extension);
if (new_songs_info_.isEmpty()) {
ok = false;
}
}
else {
new_songs_info_.clear();
}
// Update the previews
ui_->preview->clear();
@ -532,7 +544,11 @@ void OrganizeDialog::UpdatePreviews() {
if (has_local_destination) {
for (const Organize::NewSongInfo &song_info : new_songs_info_) {
QString filename = storage->LocalPath() + "/" + song_info.new_filename_;
ui_->preview->addItem(QDir::toNativeSeparators(filename));
QListWidgetItem *item = new QListWidgetItem(song_info.unique_filename_ ? IconLoader::Load("dialog-ok-apply") : IconLoader::Load("dialog-warning"), QDir::toNativeSeparators(filename), ui_->preview);
ui_->preview->addItem(item);
if (!song_info.unique_filename_) {
ok = false;
}
}
}
@ -540,6 +556,8 @@ void OrganizeDialog::UpdatePreviews() {
AdjustSize();
}
ui_->button_box->button(QDialogButtonBox::Ok)->setEnabled(ok);
}
void OrganizeDialog::OrganizeFinished(const QStringList &files_with_errors, const QStringList &log) {

View File

@ -105,8 +105,8 @@ class OrganizeDialog : public QDialog {
void AllowExtASCII(const bool checked);
private:
static const char *kSettingsGroup;
static const char *kDefaultFormat;
static const char kSettingsGroup[];
static const char kDefaultFormat[];
QWidget *parentwindow_;
Ui_OrganizeDialog *ui_;

View File

@ -44,8 +44,16 @@
#include "organizeformat.h"
const char *OrganizeFormat::kTagPattern = "\\%([a-zA-Z]*)";
const char *OrganizeFormat::kBlockPattern = "\\{([^{}]+)\\}";
const QRegularExpression OrganizeFormat::kProblematicCharacters("[:?*\"<>|]");
// From http://en.wikipedia.org/wiki/8.3_filename#Directory_table
const QRegularExpression OrganizeFormat::kInvalidFatCharacters("[^a-zA-Z0-9!#\\$%&'()\\-@\\^_`{}~/. ]", QRegularExpression::CaseInsensitiveOption);
const QRegularExpression OrganizeFormat::kInvalidDirCharacters("[/\\\\]");
constexpr char OrganizeFormat::kInvalidPrefixCharacters[] = ".";
constexpr int OrganizeFormat::kInvalidPrefixCharactersCount = arraysize(OrganizeFormat::kInvalidPrefixCharacters) - 1;
constexpr char OrganizeFormat::kBlockPattern[] = "\\{([^{}]+)\\}";
constexpr char OrganizeFormat::kTagPattern[] = "\\%([a-zA-Z]*)";
const QStringList OrganizeFormat::kKnownTags = QStringList() << "title"
<< "album"
<< "artist"
@ -67,13 +75,8 @@ const QStringList OrganizeFormat::kKnownTags = QStringList() << "title"
<< "grouping"
<< "lyrics";
const QRegularExpression OrganizeFormat::kInvalidDirCharacters("[/\\\\]");
const QRegularExpression OrganizeFormat::kProblematicCharacters("[:?*\"<>|]");
// From http://en.wikipedia.org/wiki/8.3_filename#Directory_table
const QRegularExpression OrganizeFormat::kInvalidFatCharacters("[^a-zA-Z0-9!#\\$%&'()\\-@\\^_`{}~/. ]", QRegularExpression::CaseInsensitiveOption);
const char OrganizeFormat::kInvalidPrefixCharacters[] = ".";
const int OrganizeFormat::kInvalidPrefixCharactersCount = arraysize(OrganizeFormat::kInvalidPrefixCharacters) - 1;
const QStringList OrganizeFormat::kUniqueTags = QStringList() << "title"
<< "track";
const QRgb OrganizeFormat::SyntaxHighlighter::kValidTagColorLight = qRgb(64, 64, 255);
const QRgb OrganizeFormat::SyntaxHighlighter::kInvalidTagColorLight = qRgb(255, 64, 64);
@ -106,34 +109,48 @@ bool OrganizeFormat::IsValid() const {
}
QString OrganizeFormat::GetFilenameForSong(const Song &song, QString extension) const {
OrganizeFormat::GetFilenameForSongResult OrganizeFormat::GetFilenameForSong(const Song &song, QString extension) const {
QString filename = ParseBlock(format_, song);
QString format_path;
QString format_filename;
if (QFileInfo(filename).completeBaseName().isEmpty()) {
// Avoid having empty filenames, or filenames with extension only: in this case, keep the original filename.
// We remove the extension from "filename" if it exists, as song.basefilename() also contains the extension.
QString path = QFileInfo(filename).path();
filename.clear();
if (!path.isEmpty()) {
filename.append(path);
if (path.right(1) != '/' && path.right(1) != '\\') {
filename.append('/');
}
}
filename.append(song.basefilename());
if (format_.contains('/')) {
format_path = format_.section('/', 0, -2);
format_filename = format_.section('/', -1);
}
else {
format_filename = format_;
}
if (remove_problematic_) filename = filename.remove(kProblematicCharacters);
if (remove_non_fat_ || (remove_non_ascii_ && !allow_ascii_ext_)) filename = Utilities::Transliterate(filename);
if (remove_non_fat_) filename = filename.remove(kInvalidFatCharacters);
QString path;
if (!format_path.isEmpty()) {
path = ParseBlock(format_path, song);
}
bool unique_filename = false;
QString filename = ParseBlock(format_filename, song, &unique_filename);
if (filename.isEmpty()) {
return GetFilenameForSongResult();
}
QString filepath;
if (path.isEmpty()) {
filepath = filename;
}
else {
filepath = path + "/" + filename;
}
if (remove_problematic_) filepath = filepath.remove(kProblematicCharacters);
if (remove_non_fat_ || (remove_non_ascii_ && !allow_ascii_ext_)) filepath = Utilities::Transliterate(filepath);
if (remove_non_fat_) filepath = filepath.remove(kInvalidFatCharacters);
if (remove_non_ascii_) {
int ascii = 128;
if (allow_ascii_ext_) ascii = 255;
QString stripped;
for (int i = 0; i < filename.length(); ++i) {
const QChar c = filename[i];
for (int i = 0; i < filepath.length(); ++i) {
const QChar c = filepath[i];
if (c.unicode() < ascii) {
stripped.append(c);
}
@ -144,15 +161,23 @@ QString OrganizeFormat::GetFilenameForSong(const Song &song, QString extension)
}
}
}
filename = stripped;
filepath = stripped;
}
// Remove repeated whitespaces in the filename.
filename = filename.simplified();
// Remove repeated whitespaces in the filepath.
filepath = filepath.simplified();
QFileInfo info(filename);
if (extension.isEmpty()) extension = info.suffix();
QString filepath;
// Fixup extension
QFileInfo info(filepath);
filepath.clear();
if (extension.isEmpty()) {
if (info.completeSuffix().isEmpty()) {
extension = QFileInfo(song.url().toLocalFile()).completeSuffix();
}
else {
extension = info.completeSuffix();
}
}
if (!info.path().isEmpty() && info.path() != ".") {
filepath.append(info.path());
filepath.append("/");
@ -173,31 +198,29 @@ QString OrganizeFormat::GetFilenameForSong(const Song &song, QString extension)
part = part.trimmed();
parts_new.append(part);
}
filename = parts_new.join("/");
filepath = parts_new.join("/");
if (replace_spaces_) filename.replace(QRegularExpression("\\s"), "_");
if (replace_spaces_) filepath.replace(QRegularExpression("\\s"), "_");
if (!extension.isEmpty()) {
filename.append(QString(".%1").arg(extension));
filepath.append(QString(".%1").arg(extension));
}
return filename;
return GetFilenameForSongResult(filepath, unique_filename);
}
QString OrganizeFormat::ParseBlock(QString block, const Song &song, bool *any_empty) const {
QRegularExpression tag_regexp(kTagPattern);
QRegularExpression block_regexp(kBlockPattern);
QString OrganizeFormat::ParseBlock(QString block, const Song &song, bool *have_tagdata, bool *any_empty) const {
// Find any blocks first
qint64 pos = 0;
const QRegularExpression block_regexp(kBlockPattern);
QRegularExpressionMatch re_match;
for (re_match = block_regexp.match(block, pos); re_match.hasMatch(); re_match = block_regexp.match(block, pos)) {
pos = re_match.capturedStart();
// Recursively parse the block
bool empty = false;
QString value = ParseBlock(re_match.captured(1), song, &empty);
QString value = ParseBlock(re_match.captured(1), song, have_tagdata, &empty);
if (empty) value = "";
// Replace the block's value
@ -208,16 +231,26 @@ QString OrganizeFormat::ParseBlock(QString block, const Song &song, bool *any_em
// Now look for tags
bool empty = false;
pos = 0;
const QRegularExpression tag_regexp(kTagPattern);
for (re_match = tag_regexp.match(block, pos); re_match.hasMatch(); re_match = tag_regexp.match(block, pos)) {
pos = re_match.capturedStart();
QString value = TagValue(re_match.captured(1), song);
if (value.isEmpty()) empty = true;
const QString tag = re_match.captured(1);
const QString value = TagValue(tag, song);
if (value.isEmpty()) {
empty = true;
}
else if (have_tagdata && kUniqueTags.contains(tag)) {
*have_tagdata = true;
}
block.replace(pos, re_match.capturedLength(), value);
pos += value.length();
}
if (any_empty) *any_empty = empty;
if (any_empty) {
*any_empty = empty;
}
return block;
}
@ -309,8 +342,6 @@ OrganizeFormat::Validator::Validator(QObject *parent) : QValidator(parent) {}
QValidator::State OrganizeFormat::Validator::validate(QString &input, int&) const {
QRegularExpression tag_regexp(kTagPattern);
// Make sure all the blocks match up
int block_level = 0;
for (int i = 0; i < input.length(); ++i) {
@ -327,6 +358,7 @@ QValidator::State OrganizeFormat::Validator::validate(QString &input, int&) cons
if (block_level != 0) return QValidator::Invalid;
// Make sure the tags are valid
const QRegularExpression tag_regexp(kTagPattern);
QRegularExpressionMatch re_match;
qint64 pos = 0;
for (re_match = tag_regexp.match(input, pos); re_match.hasMatch(); re_match = tag_regexp.match(input, pos)) {
@ -355,9 +387,6 @@ void OrganizeFormat::SyntaxHighlighter::highlightBlock(const QString &text) {
const QRgb valid_tag_color = light ? kValidTagColorLight : kValidTagColorDark;
const QRgb invalid_tag_color = light ? kInvalidTagColorLight : kInvalidTagColorDark;
QRegularExpression tag_regexp(kTagPattern);
QRegularExpression block_regexp(kBlockPattern);
QTextCharFormat block_format;
block_format.setBackground(QColor(block_color));
@ -365,6 +394,7 @@ void OrganizeFormat::SyntaxHighlighter::highlightBlock(const QString &text) {
setFormat(0, static_cast<int>(text.length()), QTextCharFormat());
// Blocks
const QRegularExpression block_regexp(kBlockPattern);
QRegularExpressionMatch re_match;
qint64 pos = 0;
for (re_match = block_regexp.match(text, pos); re_match.hasMatch(); re_match = block_regexp.match(text, pos)) {
@ -374,6 +404,7 @@ void OrganizeFormat::SyntaxHighlighter::highlightBlock(const QString &text) {
}
// Tags
const QRegularExpression tag_regexp(kTagPattern);
pos = 0;
for (re_match = tag_regexp.match(text, pos); re_match.hasMatch(); re_match = tag_regexp.match(text, pos)) {
pos = re_match.capturedStart();

View File

@ -41,16 +41,9 @@ class OrganizeFormat {
public:
explicit OrganizeFormat(const QString &format = QString());
static const char *kTagPattern;
static const char *kBlockPattern;
static const QStringList kKnownTags;
static const QRegularExpression kInvalidDirCharacters;
static const QRegularExpression kProblematicCharacters;
static const QRegularExpression kInvalidFatCharacters;
static const char kInvalidPrefixCharacters[];
static const int kInvalidPrefixCharactersCount;
QString format() const { return format_; }
bool remove_problematic() const { return remove_problematic_; }
bool remove_non_fat() const { return remove_non_fat_; }
@ -66,7 +59,13 @@ class OrganizeFormat {
void set_replace_spaces(const bool v) { replace_spaces_ = v; }
bool IsValid() const;
QString GetFilenameForSong(const Song &song, QString extension = QString()) const;
struct GetFilenameForSongResult {
GetFilenameForSongResult(const QString &_filename = QString(), const bool _unique_filename = false) : filename(_filename), unique_filename(_unique_filename) {}
QString filename;
bool unique_filename;
};
GetFilenameForSongResult GetFilenameForSong(const Song& song, QString extension = QString()) const;
class Validator : public QValidator { // clazy:exclude=missing-qobject-macro
public:
@ -90,7 +89,15 @@ class OrganizeFormat {
};
private:
QString ParseBlock(QString block, const Song &song, bool *any_empty = nullptr) const;
static const char kBlockPattern[];
static const char kTagPattern[];
static const QStringList kKnownTags;
static const QStringList kUniqueTags;
static const QRegularExpression kInvalidDirCharacters;
static const char kInvalidPrefixCharacters[];
static const int kInvalidPrefixCharactersCount;
QString ParseBlock(QString block, const Song &song, bool *have_tagdata = nullptr, bool *any_empty = nullptr) const;
QString TagValue(const QString &tag, const Song &song) const;
QString format_;

View File

@ -62,7 +62,7 @@ TEST_F(OrganizeFormatTest, BasicReplace) {
ASSERT_TRUE(format_.IsValid());
EXPECT_EQ("album_albumartist_artist_123_comment_composer_performer_grouping_789_genre_987_654_32_title_321_2010", format_.GetFilenameForSong(song_));
EXPECT_EQ("album_albumartist_artist_123_comment_composer_performer_grouping_789_genre_987_654_32_title_321_2010", format_.GetFilenameForSong(song_).filename);
}
@ -78,35 +78,7 @@ TEST_F(OrganizeFormatTest, BasicReplacePaths) {
ASSERT_TRUE(format_.IsValid());
EXPECT_EQ("albumartist/album/321_albumartist_artist_album_title", format_.GetFilenameForSong(song_));
}
TEST_F(OrganizeFormatTest, PathOnlyFormat) {
song_.set_title("title");
song_.set_album("album");
song_.set_artist("artist");
song_.set_albumartist("albumartist");
song_.set_track(321);
song_.set_url(QUrl("file:///music/whatever/321_albumartist_artist_album_title"));
song_.set_basefilename("321_albumartist_artist_album_title");
format_.set_format("%albumartist/%album/");
ASSERT_TRUE(format_.IsValid());
EXPECT_EQ("albumartist/album/321_albumartist_artist_album_title", format_.GetFilenameForSong(song_));
}
TEST_F(OrganizeFormatTest, Extension) {
song_.set_url(QUrl("file:///some/path/filename.flac"));
format_.set_format("%extension");
ASSERT_TRUE(format_.IsValid());
EXPECT_EQ("flac", format_.GetFilenameForSong(song_));
EXPECT_EQ("albumartist/album/321_albumartist_artist_album_title", format_.GetFilenameForSong(song_).filename);
}
@ -116,7 +88,7 @@ TEST_F(OrganizeFormatTest, ArtistInitial) {
format_.set_format("%artistinitial");
ASSERT_TRUE(format_.IsValid());
EXPECT_EQ("B", format_.GetFilenameForSong(song_));
EXPECT_EQ("B", format_.GetFilenameForSong(song_).filename);
}
@ -126,7 +98,7 @@ TEST_F(OrganizeFormatTest, AlbumArtistInitial) {
format_.set_format("%artistinitial");
ASSERT_TRUE(format_.IsValid());
EXPECT_EQ("B", format_.GetFilenameForSong(song_));
EXPECT_EQ("B", format_.GetFilenameForSong(song_).filename);
}
@ -143,13 +115,13 @@ TEST_F(OrganizeFormatTest, Blocks) {
ASSERT_TRUE(format_.IsValid());
song_.set_year(-1);
EXPECT_EQ("BeforeAfter", format_.GetFilenameForSong(song_));
EXPECT_EQ("BeforeAfter", format_.GetFilenameForSong(song_).filename);
song_.set_year(0);
EXPECT_EQ("BeforeAfter", format_.GetFilenameForSong(song_));
EXPECT_EQ("BeforeAfter", format_.GetFilenameForSong(song_).filename);
song_.set_year(123);
EXPECT_EQ("BeforeInside123After", format_.GetFilenameForSong(song_));
EXPECT_EQ("BeforeInside123After", format_.GetFilenameForSong(song_).filename);
}
@ -159,9 +131,9 @@ TEST_F(OrganizeFormatTest, ReplaceSpaces) {
format_.set_format("The Format String %title");
format_.set_replace_spaces(false);
EXPECT_EQ("The Format String The Song Title", format_.GetFilenameForSong(song_));
EXPECT_EQ("The Format String The Song Title", format_.GetFilenameForSong(song_).filename);
format_.set_replace_spaces(true);
EXPECT_EQ("The_Format_String_The_Song_Title", format_.GetFilenameForSong(song_));
EXPECT_EQ("The_Format_String_The_Song_Title", format_.GetFilenameForSong(song_).filename);
}
@ -171,21 +143,21 @@ TEST_F(OrganizeFormatTest, ReplaceNonAscii) {
format_.set_format("%artist");
format_.set_remove_non_ascii(false);
EXPECT_EQ(QString::fromUtf8("Röyksopp"), format_.GetFilenameForSong(song_));
EXPECT_EQ(QString::fromUtf8("Röyksopp"), format_.GetFilenameForSong(song_).filename);
format_.set_remove_non_ascii(true);
EXPECT_EQ("Royksopp", format_.GetFilenameForSong(song_));
EXPECT_EQ("Royksopp", format_.GetFilenameForSong(song_).filename);
song_.set_artist("");
EXPECT_EQ("", format_.GetFilenameForSong(song_));
EXPECT_EQ("", format_.GetFilenameForSong(song_).filename);
#ifdef HAVE_ICU
song_.set_artist(QString::fromUtf8("Владимир Высоцкий"));
EXPECT_EQ("Vladimir_Vysockij", format_.GetFilenameForSong(song_));
EXPECT_EQ("Vladimir_Vysockij", format_.GetFilenameForSong(song_).filename);
song_.set_artist(QString::fromUtf8("エックス・ジャパン"));
EXPECT_EQ("ekkusujapan", format_.GetFilenameForSong(song_));
EXPECT_EQ("ekkusujapan", format_.GetFilenameForSong(song_).filename);
#endif
@ -196,16 +168,16 @@ TEST_F(OrganizeFormatTest, TrackNumberPadding) {
format_.set_format("%track");
song_.set_track(9);
EXPECT_EQ("09", format_.GetFilenameForSong(song_));
EXPECT_EQ("09", format_.GetFilenameForSong(song_).filename);
song_.set_track(99);
EXPECT_EQ("99", format_.GetFilenameForSong(song_));
EXPECT_EQ("99", format_.GetFilenameForSong(song_).filename);
song_.set_track(999);
EXPECT_EQ("999", format_.GetFilenameForSong(song_));
EXPECT_EQ("999", format_.GetFilenameForSong(song_).filename);
song_.set_track(0);
EXPECT_EQ("", format_.GetFilenameForSong(song_));
EXPECT_EQ("", format_.GetFilenameForSong(song_).filename);
}
@ -213,6 +185,6 @@ TEST_F(OrganizeFormatTest, ReplaceSlashes) {
format_.set_format("%title");
song_.set_title("foo/bar\\baz");
EXPECT_EQ("foobarbaz", format_.GetFilenameForSong(song_));
EXPECT_EQ("foobarbaz", format_.GetFilenameForSong(song_).filename);
}