From ecd3b87cc03553b26ea8f26d9ea89dbb54596b6a Mon Sep 17 00:00:00 2001 From: Dan McCarthy Date: Fri, 29 Dec 2023 20:01:30 -0800 Subject: [PATCH] Debugger: Fix Importing Breakpoints CSV functionality (#10486) * Debugger: Fix Importing Breakpoints CSV functionality This resolves a few issues with the Breakpoint CSV importing functionality. Makes the following changes/fixes: - Assembly instructions with commas caused part of the instruction text to be considered the start of a new column (comma is the CSV delimiter), resulting in "too many columns errors" and failing to import. Now puts values in quotes and detects the true start and end of the values, so that no extra columns are created. - The import function was using the incorrect columns to load since the Enabled column was moved to be the first one but the import function had not been updated. Fixed this by using the column enum values instead of hardcoded numbers, so if ordering in the model changes it will still import fine. - Updates the beginRemoveRows function to not remove an extra row. With this function you would want a "count" of 0 to remove 1, so "row, row" deletes one, "row, row+1" deletes 2. Updates it to subtract one so that the range is not including one more than it is intended to. - * Misc: Cleanup regex string & add explicit imports Uses raw string literal to make the regular expression easier to read since regex is ugly enough as it is, we really don't need escaping characters in a string on top. Also explicitly includes used Qt classes. --- pcsx2-qt/Debugger/CpuWidget.cpp | 52 +++++++++++++------- pcsx2-qt/Debugger/Models/BreakpointModel.cpp | 2 +- pcsx2-qt/QtUtils.cpp | 10 ++-- pcsx2-qt/QtUtils.h | 2 +- 4 files changed, 42 insertions(+), 24 deletions(-) diff --git a/pcsx2-qt/Debugger/CpuWidget.cpp b/pcsx2-qt/Debugger/CpuWidget.cpp index b2339af733..b789f6f9e8 100644 --- a/pcsx2-qt/Debugger/CpuWidget.cpp +++ b/pcsx2-qt/Debugger/CpuWidget.cpp @@ -23,6 +23,9 @@ #include #include #include +#include +#include +#include #include using namespace QtUtils; @@ -312,7 +315,7 @@ void CpuWidget::onBPListContextMenu(QPoint pos) QAction* actionExport = new QAction(tr("Copy all as CSV"), m_ui.breakpointList); connect(actionExport, &QAction::triggered, [this]() { // It's important to use the Export Role here to allow pasting to be translation agnostic - QGuiApplication::clipboard()->setText(QtUtils::AbstractItemModelToCSV(m_ui.breakpointList->model(), BreakpointModel::ExportRole)); + QGuiApplication::clipboard()->setText(QtUtils::AbstractItemModelToCSV(m_ui.breakpointList->model(), BreakpointModel::ExportRole, true)); }); contextMenu->addAction(actionExport); @@ -381,7 +384,18 @@ void CpuWidget::contextBPListPasteCSV() for (const QString& line : csv.split('\n')) { - const QStringList fields = line.split(','); + QStringList fields; + // In order to handle text with commas in them we must wrap values in quotes to mark + // where a value starts and end so that text commas aren't identified as delimiters. + // So matches each quote pair, parse it out, and removes the quotes to get the value. + QRegularExpression eachQuotePair(R"("([^"]|\\.)*")"); + QRegularExpressionMatchIterator it = eachQuotePair.globalMatch(line); + while (it.hasNext()) + { + QRegularExpressionMatch match = it.next(); + QString matchedValue = match.captured(0); + fields << matchedValue.mid(1, matchedValue.length() - 2); + } if (fields.size() != BreakpointModel::BreakpointColumns::COLUMN_COUNT) { Console.WriteLn("Debugger CSV Import: Invalid number of columns, skipping"); @@ -389,10 +403,10 @@ void CpuWidget::contextBPListPasteCSV() } bool ok; - int type = fields[0].toUInt(&ok); + int type = fields[BreakpointModel::BreakpointColumns::TYPE].toUInt(&ok); if (!ok) { - Console.WriteLn("Debugger CSV Import: Failed to parse type '%s', skipping", fields[0].toUtf8().constData()); + Console.WriteLn("Debugger CSV Import: Failed to parse type '%s', skipping", fields[BreakpointModel::BreakpointColumns::TYPE].toUtf8().constData()); continue; } @@ -402,34 +416,34 @@ void CpuWidget::contextBPListPasteCSV() BreakPoint bp; // Address - bp.addr = fields[1].toUInt(&ok, 16); + bp.addr = fields[BreakpointModel::BreakpointColumns::OFFSET].toUInt(&ok, 16); if (!ok) { - Console.WriteLn("Debugger CSV Import: Failed to parse address '%s', skipping", fields[1].toUtf8().constData()); + Console.WriteLn("Debugger CSV Import: Failed to parse address '%s', skipping", fields[BreakpointModel::BreakpointColumns::OFFSET].toUtf8().constData()); continue; } // Condition - if (!fields[4].isEmpty()) + if (!fields[BreakpointModel::BreakpointColumns::CONDITION].isEmpty()) { PostfixExpression expr; bp.hasCond = true; bp.cond.debug = &m_cpu; - if (!m_cpu.initExpression(fields[4].toUtf8().constData(), expr)) + if (!m_cpu.initExpression(fields[BreakpointModel::BreakpointColumns::CONDITION].toUtf8().constData(), expr)) { - Console.WriteLn("Debugger CSV Import: Failed to parse cond '%s', skipping", fields[4].toUtf8().constData()); + Console.WriteLn("Debugger CSV Import: Failed to parse cond '%s', skipping", fields[BreakpointModel::BreakpointColumns::CONDITION].toUtf8().constData()); continue; } bp.cond.expression = expr; - strncpy(&bp.cond.expressionString[0], fields[4].toUtf8().constData(), sizeof(bp.cond.expressionString)); + strncpy(&bp.cond.expressionString[0], fields[BreakpointModel::BreakpointColumns::CONDITION].toUtf8().constData(), sizeof(bp.cond.expressionString)); } // Enabled - bp.enabled = fields[6].toUInt(&ok); + bp.enabled = fields[BreakpointModel::BreakpointColumns::ENABLED].toUInt(&ok); if (!ok) { - Console.WriteLn("Debugger CSV Import: Failed to parse enable flag '%s', skipping", fields[1].toUtf8().constData()); + Console.WriteLn("Debugger CSV Import: Failed to parse enable flag '%s', skipping", fields[BreakpointModel::BreakpointColumns::ENABLED].toUtf8().constData()); continue; } @@ -441,32 +455,32 @@ void CpuWidget::contextBPListPasteCSV() // Mode if (type >= MEMCHECK_INVALID) { - Console.WriteLn("Debugger CSV Import: Failed to parse cond type '%s', skipping", fields[0].toUtf8().constData()); + Console.WriteLn("Debugger CSV Import: Failed to parse cond type '%s', skipping", fields [BreakpointModel::BreakpointColumns::TYPE].toUtf8().constData()); continue; } mc.cond = static_cast(type); // Address - mc.start = fields[1].toUInt(&ok, 16); + mc.start = fields[BreakpointModel::BreakpointColumns::OFFSET].toUInt(&ok, 16); if (!ok) { - Console.WriteLn("Debugger CSV Import: Failed to parse address '%s', skipping", fields[1].toUtf8().constData()); + Console.WriteLn("Debugger CSV Import: Failed to parse address '%s', skipping", fields[BreakpointModel::BreakpointColumns::OFFSET].toUtf8().constData()); continue; } // Size - mc.end = fields[2].toUInt(&ok) + mc.start; + mc.end = fields[BreakpointModel::BreakpointColumns::SIZE_LABEL].toUInt(&ok) + mc.start; if (!ok) { - Console.WriteLn("Debugger CSV Import: Failed to parse length '%s', skipping", fields[1].toUtf8().constData()); + Console.WriteLn("Debugger CSV Import: Failed to parse length '%s', skipping", fields[BreakpointModel::BreakpointColumns::SIZE_LABEL].toUtf8().constData()); continue; } // Result - int result = fields[6].toUInt(&ok); + int result = fields [BreakpointModel::BreakpointColumns::ENABLED].toUInt(&ok); if (!ok) { - Console.WriteLn("Debugger CSV Import: Failed to parse result flag '%s', skipping", fields[1].toUtf8().constData()); + Console.WriteLn("Debugger CSV Import: Failed to parse result flag '%s', skipping", fields [BreakpointModel::BreakpointColumns::ENABLED].toUtf8().constData()); continue; } mc.result = static_cast(result); diff --git a/pcsx2-qt/Debugger/Models/BreakpointModel.cpp b/pcsx2-qt/Debugger/Models/BreakpointModel.cpp index 8e06100628..3e9fe8d797 100644 --- a/pcsx2-qt/Debugger/Models/BreakpointModel.cpp +++ b/pcsx2-qt/Debugger/Models/BreakpointModel.cpp @@ -322,7 +322,7 @@ bool BreakpointModel::setData(const QModelIndex& index, const QVariant& value, i bool BreakpointModel::removeRows(int row, int count, const QModelIndex& index) { - beginRemoveRows(index, row, row + count); + beginRemoveRows(index, row, row + count - 1); for (int i = row; i < row + count; i++) { diff --git a/pcsx2-qt/QtUtils.cpp b/pcsx2-qt/QtUtils.cpp index a326a5814e..b09737d2cf 100644 --- a/pcsx2-qt/QtUtils.cpp +++ b/pcsx2-qt/QtUtils.cpp @@ -255,13 +255,15 @@ namespace QtUtils return wi; } - QString AbstractItemModelToCSV(QAbstractItemModel* model, int role) + QString AbstractItemModelToCSV(QAbstractItemModel* model, int role, bool useQuotes) { QString csv; // Header for (int col = 0; col < model->columnCount(); col++) { - csv += model->headerData(col, Qt::Horizontal, Qt::DisplayRole).toString(); + // Encapsulate value in quotes so that commas don't break the column count. + QString headerLine = model->headerData(col, Qt::Horizontal, Qt::DisplayRole).toString(); + csv += useQuotes ? QString("\"%1\"").arg(headerLine) : headerLine; if (col < model->columnCount() - 1) csv += ","; } @@ -273,7 +275,9 @@ namespace QtUtils { for (int col = 0; col < model->columnCount(); col++) { - csv += model->data(model->index(row, col), role).toString(); + // Encapsulate value in quotes so that commas don't break the column count. + QString dataLine = model->data(model->index(row, col), role).toString(); + csv += useQuotes ? QString("\"%1\"").arg(dataLine) : dataLine; if (col < model->columnCount() - 1) csv += ","; diff --git a/pcsx2-qt/QtUtils.h b/pcsx2-qt/QtUtils.h index 081aa02647..651cfe426b 100644 --- a/pcsx2-qt/QtUtils.h +++ b/pcsx2-qt/QtUtils.h @@ -84,5 +84,5 @@ namespace QtUtils }; /// Converts an abstract item model to a CSV string. - QString AbstractItemModelToCSV(QAbstractItemModel* model, int role = Qt::DisplayRole); + QString AbstractItemModelToCSV(QAbstractItemModel* model, int role = Qt::DisplayRole, bool useQuotes = false); } // namespace QtUtils