diff --git a/CMakeLists.txt b/CMakeLists.txt index 4fc09271f3f..29b6ec35088 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -877,6 +877,7 @@ add_library(mixxx-lib STATIC EXCLUDE_FROM_ALL src/util/mac.cpp src/util/movinginterquartilemean.cpp src/util/performancetimer.cpp + src/util/rangelist.cpp src/util/readaheadsamplebuffer.cpp src/util/rlimit.cpp src/util/rotary.cpp @@ -1523,6 +1524,7 @@ add_executable(mixxx-test src/test/portmidicontroller_test.cpp src/test/portmidienumeratortest.cpp src/test/queryutiltest.cpp + src/test/rangelist_test.cpp src/test/readaheadmanager_test.cpp src/test/replaygaintest.cpp src/test/rescalertest.cpp diff --git a/src/preferences/colorpaletteeditor.cpp b/src/preferences/colorpaletteeditor.cpp index 1d3408787f1..00fb53c63a4 100644 --- a/src/preferences/colorpaletteeditor.cpp +++ b/src/preferences/colorpaletteeditor.cpp @@ -186,7 +186,7 @@ void ColorPaletteEditor::slotTableViewDoubleClicked(const QModelIndex& index) { } void ColorPaletteEditor::slotAddColor() { - m_pModel->appendRow(kDefaultPaletteColor); + m_pModel->appendRow(kDefaultPaletteColor, {}); m_pTableView->scrollToBottom(); m_pTableView->setCurrentIndex( m_pModel->index(m_pModel->rowCount() - 1, 0)); diff --git a/src/preferences/colorpaletteeditormodel.cpp b/src/preferences/colorpaletteeditormodel.cpp index d0f2d1a4255..dfc01b3bf8f 100644 --- a/src/preferences/colorpaletteeditormodel.cpp +++ b/src/preferences/colorpaletteeditormodel.cpp @@ -1,5 +1,14 @@ #include "preferences/colorpaletteeditormodel.h" +#include +#include + +#include +#include +#include +#include + +#include "engine/controls/cuecontrol.h" #include "moc_colorpaletteeditormodel.cpp" namespace { @@ -10,6 +19,13 @@ QIcon toQIcon(const QColor& color) { return QIcon(pixmap); } +HotcueIndexListItem* toHotcueIndexListItem(QStandardItem* pFrom) { + VERIFY_OR_DEBUG_ASSERT(pFrom->type() == QStandardItem::UserType) { + return nullptr; + } + return static_cast(pFrom); +} + } // namespace ColorPaletteEditorModel::ColorPaletteEditorModel(QObject* parent) @@ -53,25 +69,39 @@ bool ColorPaletteEditorModel::dropMimeData(const QMimeData* data, Qt::DropAction bool ColorPaletteEditorModel::setData(const QModelIndex& modelIndex, const QVariant& value, int role) { setDirty(true); if (modelIndex.isValid() && modelIndex.column() == 1) { - bool ok; - int hotcueIndex = value.toInt(&ok); + const bool initialAttemptSuccessful = QStandardItemModel::setData(modelIndex, value, role); - // Make sure that the value is valid - if (!ok || hotcueIndex <= 0 || hotcueIndex > rowCount()) { - return QStandardItemModel::setData(modelIndex, QVariant(), role); + const auto* pHotcueIndexListItem = toHotcueIndexListItem(itemFromIndex(modelIndex)); + VERIFY_OR_DEBUG_ASSERT(pHotcueIndexListItem) { + return false; } - // Make sure there is no other row with the same hotcue index - for (int i = 0; i < rowCount(); i++) { - QModelIndex otherModelIndex = index(i, 1); - QVariant otherValue = data(otherModelIndex); - int otherHotcueIndex = otherValue.toInt(&ok); - if (ok && otherHotcueIndex == hotcueIndex) { - QStandardItemModel::setData(otherModelIndex, QVariant(), role); + auto hotcueIndexList = pHotcueIndexListItem->getHotcueIndexList(); + + // make sure no index is outside of range + DEBUG_ASSERT(std::is_sorted(hotcueIndexList.cbegin(), hotcueIndexList.cend())); + auto endUpper = std::upper_bound( + hotcueIndexList.begin(), hotcueIndexList.end(), NUM_HOT_CUES); + hotcueIndexList.erase(endUpper, hotcueIndexList.end()); + auto endLower = std::upper_bound(hotcueIndexList.begin(), hotcueIndexList.end(), 0); + hotcueIndexList.erase(hotcueIndexList.begin(), endLower); + + for (int i = 0; i < rowCount(); ++i) { + auto* pHotcueIndexListItem = toHotcueIndexListItem(item(i, 1)); + + if (pHotcueIndexListItem == nullptr) { + continue; + } + + if (i == modelIndex.row()) { + pHotcueIndexListItem->setHotcueIndexList(hotcueIndexList); + } else { + pHotcueIndexListItem->removeIndicies(hotcueIndexList); } } - } + return initialAttemptSuccessful; + } return QStandardItemModel::setData(modelIndex, value, role); } @@ -84,21 +114,18 @@ void ColorPaletteEditorModel::setColor(int row, const QColor& color) { setDirty(true); } -void ColorPaletteEditorModel::appendRow(const QColor& color, int hotcueIndex) { +void ColorPaletteEditorModel::appendRow( + const QColor& color, const QList& hotcueIndicies) { QStandardItem* pColorItem = new QStandardItem(toQIcon(color), color.name()); pColorItem->setEditable(false); pColorItem->setDropEnabled(false); - QString hotcueIndexStr; - if (hotcueIndex >= 0) { - hotcueIndexStr = QString::number(hotcueIndex + 1); - } - - QStandardItem* pHotcueIndexItem = new QStandardItem(hotcueIndexStr); + QStandardItem* pHotcueIndexItem = new HotcueIndexListItem(hotcueIndicies); pHotcueIndexItem->setEditable(true); pHotcueIndexItem->setDropEnabled(false); - QStandardItemModel::appendRow(QList{pColorItem, pHotcueIndexItem}); + QStandardItemModel::appendRow( + QList{pColorItem, pHotcueIndexItem}); } void ColorPaletteEditorModel::setColorPalette(const ColorPalette& palette) { @@ -106,36 +133,43 @@ void ColorPaletteEditorModel::setColorPalette(const ColorPalette& palette) { removeRows(0, rowCount()); // Make a map of hotcue indices - QMap hotcueColorIndicesMap; + QMultiMap hotcueColorIndicesMap; QList colorIndicesByHotcue = palette.getIndicesByHotcue(); for (int i = 0; i < colorIndicesByHotcue.size(); i++) { int colorIndex = colorIndicesByHotcue.at(i); - hotcueColorIndicesMap.insert(colorIndex, i); + hotcueColorIndicesMap.insert(colorIndex, i + 1); } for (int i = 0; i < palette.size(); i++) { QColor color = mixxx::RgbColor::toQColor(palette.at(i)); - int colorIndex = hotcueColorIndicesMap.value(i, kNoHotcueIndex); + QList colorIndex = hotcueColorIndicesMap.values(i); appendRow(color, colorIndex); } setDirty(false); } -ColorPalette ColorPaletteEditorModel::getColorPalette(const QString& name) const { +ColorPalette ColorPaletteEditorModel::getColorPalette( + const QString& name) const { QList colors; QMap hotcueColorIndices; for (int i = 0; i < rowCount(); i++) { QStandardItem* pColorItem = item(i, 0); - QStandardItem* pHotcueIndexItem = item(i, 1); - mixxx::RgbColor::optional_t color = mixxx::RgbColor::fromQString(pColorItem->text()); + + const auto* pHotcueIndexItem = toHotcueIndexListItem(item(i, 1)); + if (!pHotcueIndexItem) { + continue; + } + + mixxx::RgbColor::optional_t color = + mixxx::RgbColor::fromQString(pColorItem->text()); + if (color) { + const QList hotcueIndexes = pHotcueIndexItem->getHotcueIndexList(); colors << *color; - bool ok; - int hotcueIndex = pHotcueIndexItem->text().toInt(&ok); - if (ok) { - hotcueColorIndices.insert(hotcueIndex - 1, colors.size() - 1); + for (int index : hotcueIndexes) { + hotcueColorIndices.insert(index - 1, colors.size() - 1); } } } @@ -143,3 +177,56 @@ ColorPalette ColorPaletteEditorModel::getColorPalette(const QString& name) const // due to the sorting nature of QMap. This is intended, this way we have a color for every hotcue. return ColorPalette(name, colors, hotcueColorIndices.values()); } + +HotcueIndexListItem::HotcueIndexListItem(const QList& hotcueList) + : QStandardItem(), m_hotcueIndexList(hotcueList) { + std::sort(m_hotcueIndexList.begin(), m_hotcueIndexList.end()); +} +QVariant HotcueIndexListItem::data(int role) const { + switch (role) { + case Qt::DisplayRole: + case Qt::EditRole: { + return QVariant(mixxx::stringifyRangeList(m_hotcueIndexList)); + break; + } + default: + return QStandardItem::data(role); + break; + } +} + +void HotcueIndexListItem::setData(const QVariant& value, int role) { + switch (role) { + case Qt::EditRole: { + const QList newHotcueIndicies = mixxx::parseRangeList(value.toString()); + + if (m_hotcueIndexList != newHotcueIndicies) { + m_hotcueIndexList = newHotcueIndicies; + emitDataChanged(); + } + break; + } + default: + QStandardItem::setData(value, role); + break; + } +} + +void HotcueIndexListItem::removeIndicies(const QList& otherIndicies) { + DEBUG_ASSERT(std::is_sorted(otherIndicies.cbegin(), otherIndicies.cend())); + DEBUG_ASSERT(std::is_sorted(m_hotcueIndexList.cbegin(), m_hotcueIndexList.cend())); + + QList hotcueIndiciesWithOthersRemoved; + hotcueIndiciesWithOthersRemoved.reserve(m_hotcueIndexList.size()); + + std::set_difference(m_hotcueIndexList.cbegin(), + m_hotcueIndexList.cend(), + otherIndicies.cbegin(), + otherIndicies.cend(), + std::back_inserter(hotcueIndiciesWithOthersRemoved)); + + if (m_hotcueIndexList != hotcueIndiciesWithOthersRemoved) { + m_hotcueIndexList = hotcueIndiciesWithOthersRemoved; + emitDataChanged(); + } +} diff --git a/src/preferences/colorpaletteeditormodel.h b/src/preferences/colorpaletteeditormodel.h index b76c7a0053f..d57f7ace10e 100644 --- a/src/preferences/colorpaletteeditormodel.h +++ b/src/preferences/colorpaletteeditormodel.h @@ -1,23 +1,23 @@ #pragma once +#include #include +#include #include "util/color/colorpalette.h" // Model that is used by the QTableView of the ColorPaletteEditor. -// Takes of displaying palette colors and provides a getter/setter for +// Takes care of displaying palette colors and provides a getter/setter for // ColorPalette instances. class ColorPaletteEditorModel : public QStandardItemModel { Q_OBJECT public: - static constexpr int kNoHotcueIndex = -1; - ColorPaletteEditorModel(QObject* parent = nullptr); bool dropMimeData(const QMimeData* data, Qt::DropAction action, int row, int column, const QModelIndex& parent) override; bool setData(const QModelIndex& index, const QVariant& value, int role = Qt::EditRole) override; void setColor(int row, const QColor& color); - void appendRow(const QColor& color, int hotcueIndex = kNoHotcueIndex); + void appendRow(const QColor& color, const QList& hotcueIndicies); void setDirty(bool bDirty) { if (m_bDirty == bDirty) { @@ -46,3 +46,27 @@ class ColorPaletteEditorModel : public QStandardItemModel { bool m_bEmpty; bool m_bDirty; }; + +class HotcueIndexListItem : public QStandardItem { + public: + HotcueIndexListItem(const QList& hotcueList = {}); + + void setData(const QVariant& value, int role = Qt::UserRole + 1) override; + QVariant data(int role = Qt::UserRole + 1) const override; + + int type() const override { + return QStandardItem::UserType; + }; + + const QList& getHotcueIndexList() const { + return m_hotcueIndexList; + } + void setHotcueIndexList(const QList& list) { + m_hotcueIndexList = QList(list); + } + + void removeIndicies(const QList& otherIndicies); + + private: + QList m_hotcueIndexList; +}; diff --git a/src/test/rangelist_test.cpp b/src/test/rangelist_test.cpp new file mode 100644 index 00000000000..907054b28a9 --- /dev/null +++ b/src/test/rangelist_test.cpp @@ -0,0 +1,66 @@ +#include "util/rangelist.h" + +#include + +#include +#include + +#include "test/mixxxtest.h" + +void roundTripTestStr(const QString& in, const QString* out = nullptr) { + const QString roundTrip = mixxx::stringifyRangeList(mixxx::parseRangeList(in)); + if (out == nullptr) { + EXPECT_QSTRING_EQ(in, roundTrip); + } else { + EXPECT_QSTRING_EQ(*out, roundTrip); + } +} + +void roundTripTestList(const QList& in, const QList* out = nullptr) { + const QList roundTrip = mixxx::parseRangeList(mixxx::stringifyRangeList(in)); + if (out == nullptr) { + EXPECT_EQ(in, roundTrip); + } else { + EXPECT_EQ(*out, roundTrip); + } +} + +TEST(DisplayIntListTest, ListEmpty) { + roundTripTestList({}); + roundTripTestStr(""); +} + +TEST(DisplayIntListTest, smallContinousRangeIsSeparate) { + const QList list = mixxx::parseRangeList(QStringLiteral("1 - 2")); + EXPECT_EQ(list, QList({1, 2})); + EXPECT_QSTRING_EQ("1, 2", mixxx::stringifyRangeList(list)); +} + +TEST(DisplayIntListTest, whiteSpaceAreIgnored) { + EXPECT_EQ(QList({1, 2, 3, 4}), mixxx::parseRangeList(" 1,2 , 3, 4")); +} + +TEST(DisplayIntListTest, trailingCommaIsIgnored) { + EXPECT_EQ(QList({1}), mixxx::parseRangeList("1,")); +} +TEST(DisplayIntListTest, largeRangesAreExpanded) { + EXPECT_EQ(QList({1, 2, 3, 4, 5, 6, 7}), mixxx::parseRangeList("1 - 7")); +} + +TEST(DisplayIntListTest, duplicateValuesAreIgnored) { + EXPECT_EQ(QList({1, 2, 3}), mixxx::parseRangeList("1, 1, 1, 1, 2, 2, 3")); +} + +TEST(DisplayIntListTest, resultingListIsSortedAscending) { + const auto list = mixxx::parseRangeList("4, 2, 3, 1, 6, 5"); + EXPECT_TRUE(std::is_sorted(list.cbegin(), list.cend())); +} +TEST(DisplayIntListTest, consequitiveValuesAreRanges) { + EXPECT_QSTRING_EQ("1 - 4", mixxx::stringifyRangeList(QList({1, 2, 3, 4}))); +} +TEST(DisplayIntListTest, adjacentRangeAndLiteralGetsCollapsed) { + EXPECT_EQ(QList({1, 2, 3, 4, 5}), mixxx::parseRangeList("1, 2 - 4, 5")); +} +TEST(DisplayIntListTest, overLappingRangesGetUnionized) { + EXPECT_EQ(QList({1, 2, 3, 4}), mixxx::parseRangeList("1 - 3, 2 - 4")); +} diff --git a/src/util/rangelist.cpp b/src/util/rangelist.cpp new file mode 100644 index 00000000000..23576b8e8de --- /dev/null +++ b/src/util/rangelist.cpp @@ -0,0 +1,97 @@ +#include "rangelist.h" + +#include + +#include +#include + +namespace { + +const QString kGroupSeparator = QStringLiteral(", "); +const QString kRangeSeparator = QStringLiteral(" - "); +// when changing groupSeparator or rangeSeparator, rangeListMatchingRegex must +// be adjusted as well. +const QRegularExpression kRangeListMatchingRegex( + QStringLiteral("(?:(\\d+)(?:\\s*-\\s*(\\d+))?)[\\s,]*")); + +} // namespace + +namespace mixxx { + +QList parseRangeList(const QString& input) { + QList intList; + auto matchGroups = kRangeListMatchingRegex.globalMatch(input); + while (matchGroups.hasNext()) { + const auto group = matchGroups.next(); + const QString rangeStart = group.captured(1); + const QString rangeEnd = group.captured(2); + bool startOk, endOk; + int startIndex = rangeStart.toInt(&startOk); + if (!startOk) { + continue; + } + int endIndex = rangeEnd.toInt(&endOk); + if (!endOk) { + endIndex = startIndex; + } + for (int currentIndex = startIndex; currentIndex <= endIndex; currentIndex++) { + intList.append(currentIndex); + } + } + + std::sort(intList.begin(), intList.end()); + const auto end = std::unique(intList.begin(), intList.end()); + intList.erase(end, intList.end()); + + return intList; +} + +QString stringifyRangeList(const QList& rangeList) { + DEBUG_ASSERT(std::is_sorted(rangeList.cbegin(), rangeList.cend())); + DEBUG_ASSERT([&rangeList]() { + // Immediately-invoked function expression + + // assumes list is sorted! + auto first_duplicate = std::adjacent_find(rangeList.cbegin(), rangeList.cend()); + return first_duplicate == rangeList.cend(); + }()); + DEBUG_ASSERT(std::all_of(rangeList.cbegin(), rangeList.cend(), [](int val) { + return val >= 0; + })); + + QString stringifiedRangeList; + + for (int i = 0; i < rangeList.size();) { + int rangeStartIndex = i; + int rangeStartValue = rangeList.at(i); + + while (i < rangeList.size() && rangeList.at(i) == rangeStartValue + (i - rangeStartIndex)) { + i++; + } + + int rangeEndIndex = i - 1; + + stringifiedRangeList += QString::number(rangeStartValue); + + switch (rangeEndIndex - rangeStartIndex) { + case 0: + // not a range + break; + case 1: + // treat ranges of (i..i+1) as separate groups: "i, i+1" + stringifiedRangeList += kGroupSeparator + QString::number(rangeList.at(rangeEndIndex)); + break; + default: + // range where the end is >=2 than the start + stringifiedRangeList += kRangeSeparator + QString::number(rangeList.at(rangeEndIndex)); + break; + } + + if (i < rangeList.size()) { + stringifiedRangeList += kGroupSeparator; + } + } + return stringifiedRangeList; +} + +} // namespace mixxx diff --git a/src/util/rangelist.h b/src/util/rangelist.h new file mode 100644 index 00000000000..edd54dbdbbc --- /dev/null +++ b/src/util/rangelist.h @@ -0,0 +1,18 @@ +#include +#include + +namespace mixxx { + +/// parses a comma-separated list of positive ints and range if ints (eg `n - m`) +/// and returns a sorted list of all the ints described. +/// inverse counterpart of `stringifyRangeList` +QList parseRangeList(const QString& input); + +/// take a list of positive integers and stringify them into a neat +/// user friendly representation (eg {1, 2, 3} => "1 - 3"). +/// inverse counterpart of `parseRangeList`. +/// assumes rangeList is well-formed +/// (sorted, not containing duplicates, and positive integers only) +QString stringifyRangeList(const QList& rangeList); + +} // namespace mixxx