Dynamic comment field for value editing with validation feedback

- Comment shows "Enter=Save Esc=Cancel" when valid, "! value" when invalid
- Only updates on validation state change to avoid lag
- Uses deferred timer for Scintilla document sync
- Added tests for comment echo and validation error display
This commit is contained in:
sysadmin
2026-02-03 11:35:40 -07:00
parent 2c00902020
commit a7e67b12fe
6 changed files with 197 additions and 36 deletions

View File

@@ -569,6 +569,11 @@ void RcxController::batchChangeKind(const QVector<int>& nodeIndices, NodeKind ne
} }
idSet = m_doc->tree.normalizePreferDescendants(idSet); idSet = m_doc->tree.normalizePreferDescendants(idSet);
if (idSet.isEmpty()) return; if (idSet.isEmpty()) return;
// Clear selection before batch change
m_selIds.clear();
m_anchorLine = -1;
m_doc->undoStack.beginMacro(QString("Change type of %1 nodes").arg(idSet.size())); m_doc->undoStack.beginMacro(QString("Change type of %1 nodes").arg(idSet.size()));
for (uint64_t id : idSet) { for (uint64_t id : idSet) {
int idx = m_doc->tree.indexOfId(id); int idx = m_doc->tree.indexOfId(id);

View File

@@ -473,8 +473,8 @@ enum class EditTarget { Name, Type, Value };
// Column layout constants (shared with format.cpp span computation) // Column layout constants (shared with format.cpp span computation)
inline constexpr int kFoldCol = 3; // 3-char fold indicator prefix per line inline constexpr int kFoldCol = 3; // 3-char fold indicator prefix per line
inline constexpr int kColType = 10; inline constexpr int kColType = 10;
inline constexpr int kColName = 24; inline constexpr int kColName = 22;
inline constexpr int kColValue = 22; inline constexpr int kColValue = 8;
inline constexpr int kColComment = 28; // "// Enter=Save Esc=Cancel" fits inline constexpr int kColComment = 28; // "// Enter=Save Esc=Cancel" fits
inline constexpr int kSepWidth = 2; inline constexpr int kSepWidth = 2;

View File

@@ -1,4 +1,5 @@
#include "editor.h" #include "editor.h"
#include <QDebug>
#include <Qsci/qsciscintilla.h> #include <Qsci/qsciscintilla.h>
#include <Qsci/qsciscintillabase.h> #include <Qsci/qsciscintillabase.h>
#include <Qsci/qscilexercpp.h> #include <Qsci/qscilexercpp.h>
@@ -83,7 +84,7 @@ RcxEditor::RcxEditor(QWidget* parent) : QWidget(parent) {
connect(m_sci, &QsciScintilla::textChanged, this, [this]() { connect(m_sci, &QsciScintilla::textChanged, this, [this]() {
if (!m_editState.active) return; if (!m_editState.active) return;
if (m_editState.target == EditTarget::Value) if (m_editState.target == EditTarget::Value)
validateEditLive(); QTimer::singleShot(0, this, &RcxEditor::validateEditLive);
}); });
} }
@@ -933,6 +934,15 @@ bool RcxEditor::beginInlineEdit(EditTarget target, int line) {
lm->nodeKind == NodeKind::Vec4) && lm->subLine > 0) lm->nodeKind == NodeKind::Vec4) && lm->subLine > 0)
m_editState.editKind = NodeKind::Float; m_editState.editKind = NodeKind::Float;
// Store fixed comment column position for value editing
if (target == EditTarget::Value) {
ColumnSpan cs = commentSpanFor(*lm, lineText.size());
m_editState.commentCol = cs.valid ? cs.start : -1;
m_editState.lastValidationOk = true; // original value is always valid
} else {
m_editState.commentCol = -1;
}
// Disable Scintilla undo during inline edit // Disable Scintilla undo during inline edit
m_sci->SendScintilla(QsciScintillaBase::SCI_SETUNDOCOLLECTION, (long)0); m_sci->SendScintilla(QsciScintillaBase::SCI_SETUNDOCOLLECTION, (long)0);
m_sci->SendScintilla(QsciScintillaBase::SCI_SETCARETWIDTH, 1); m_sci->SendScintilla(QsciScintillaBase::SCI_SETCARETWIDTH, 1);
@@ -958,7 +968,7 @@ bool RcxEditor::beginInlineEdit(EditTarget target, int line) {
// Show initial edit hint in comment column // Show initial edit hint in comment column
if (target == EditTarget::Value) if (target == EditTarget::Value)
setEditComment(QStringLiteral("// Enter=Save Esc=Cancel")); setEditComment(QStringLiteral("Enter=Save Esc=Cancel"));
if (target == EditTarget::Type) if (target == EditTarget::Type)
QTimer::singleShot(0, this, &RcxEditor::showTypeAutocomplete); QTimer::singleShot(0, this, &RcxEditor::showTypeAutocomplete);
@@ -1040,6 +1050,15 @@ void RcxEditor::updateEditableIndicators(int line) {
if (m_editState.active) return; if (m_editState.active) return;
if (line == m_hintLine) return; if (line == m_hintLine) return;
// No cursor hints when selection is empty (prevents desync during batch ops)
if (m_currentSelIds.isEmpty()) {
if (m_hintLine >= 0) {
clearIndicatorLine(IND_EDITABLE, m_hintLine);
m_hintLine = -1;
}
return;
}
// If new line is selected, its indicators are managed by applySelectionOverlay // If new line is selected, its indicators are managed by applySelectionOverlay
// But we still need to clear the old non-selected hint line // But we still need to clear the old non-selected hint line
const LineMeta* newLm = metaForLine(line); const LineMeta* newLm = metaForLine(line);
@@ -1105,25 +1124,34 @@ void RcxEditor::applyHoverCursor() {
// ── Live value validation ── // ── Live value validation ──
void RcxEditor::setEditComment(const QString& comment) { void RcxEditor::setEditComment(const QString& comment) {
const LineMeta* lm = metaForLine(m_editState.line); // Value edit must be active
if (!lm) return; if (m_editState.commentCol < 0) return;
// Prevent re-entrancy from textChanged signal
static bool s_updating = false;
if (s_updating) return;
s_updating = true;
// Comment is always at end of line - calculate dynamically as value length changes
QString lineText = getLineText(m_sci, m_editState.line); QString lineText = getLineText(m_sci, m_editState.line);
ColumnSpan cs = commentSpanFor(*lm, lineText.size()); int startCol = lineText.size() - kColComment;
if (!cs.valid) return; if (startCol < 0) { s_updating = false; return; }
// Pad/truncate comment to fixed width
QString padded = comment.leftJustified(kColComment, ' ').left(kColComment); QString padded = comment.leftJustified(kColComment, ' ').left(kColComment);
long posA = posFromCol(m_sci, m_editState.line, cs.start); // Use direct position calculation from line start
long posB = posFromCol(m_sci, m_editState.line, cs.start + kColComment); long lineStart = m_sci->SendScintilla(QsciScintillaBase::SCI_POSITIONFROMLINE,
if (posB <= posA) return; (unsigned long)m_editState.line);
long posA = lineStart + startCol;
long posB = lineStart + startCol + kColComment;
QByteArray utf8 = padded.toUtf8(); QByteArray utf8 = padded.toUtf8();
m_sci->SendScintilla(QsciScintillaBase::SCI_SETTARGETSTART, posA); m_sci->SendScintilla(QsciScintillaBase::SCI_SETTARGETSTART, posA);
m_sci->SendScintilla(QsciScintillaBase::SCI_SETTARGETEND, posB); m_sci->SendScintilla(QsciScintillaBase::SCI_SETTARGETEND, posB);
m_sci->SendScintilla(QsciScintillaBase::SCI_REPLACETARGET, m_sci->SendScintilla(QsciScintillaBase::SCI_REPLACETARGET,
(uintptr_t)utf8.size(), utf8.constData()); (uintptr_t)utf8.size(), utf8.constData());
s_updating = false;
} }
void RcxEditor::validateEditLive() { void RcxEditor::validateEditLive() {
@@ -1134,13 +1162,24 @@ void RcxEditor::validateEditLive() {
? lineText.mid(m_editState.spanStart, editedLen).trimmed() : QString(); ? lineText.mid(m_editState.spanStart, editedLen).trimmed() : QString();
QString errorMsg = fmt::validateValue(m_editState.editKind, text); QString errorMsg = fmt::validateValue(m_editState.editKind, text);
// Show/hide error marker (red background) and update comment const LineMeta* lm = metaForLine(m_editState.line);
if (errorMsg.isEmpty()) { const bool isSelected = lm && m_currentSelIds.contains(lm->nodeId);
const bool isValid = errorMsg.isEmpty();
// Only update comment when validation state changes (avoid lag)
const bool stateChanged = (isValid != m_editState.lastValidationOk);
m_editState.lastValidationOk = isValid;
// Show/hide error marker (red background)
// M_SELECTED has higher priority than M_ERR, so temporarily remove it when error
if (isValid) {
m_sci->markerDelete(m_editState.line, M_ERR); m_sci->markerDelete(m_editState.line, M_ERR);
setEditComment(QStringLiteral("// Enter=Save Esc=Cancel")); if (isSelected) m_sci->markerAdd(m_editState.line, M_SELECTED);
if (stateChanged) setEditComment("Enter=Save Esc=Cancel");
} else { } else {
if (isSelected) m_sci->markerDelete(m_editState.line, M_SELECTED);
m_sci->markerAdd(m_editState.line, M_ERR); m_sci->markerAdd(m_editState.line, M_ERR);
setEditComment(QStringLiteral("// ") + errorMsg); if (stateChanged) setEditComment("! " + text);
} }
} }

View File

@@ -88,6 +88,8 @@ private:
int linelenAfterReplace = 0; int linelenAfterReplace = 0;
QString original; QString original;
NodeKind editKind = NodeKind::Int32; NodeKind editKind = NodeKind::Int32;
int commentCol = -1; // fixed comment column (stored at edit start)
bool lastValidationOk = true; // track state to avoid redundant updates
}; };
InlineEditState m_editState; InlineEditState m_editState;

View File

@@ -8,7 +8,7 @@ namespace rcx::fmt {
// COL_TYPE and COL_NAME use shared constants from core.h (kColType, kColName) // COL_TYPE and COL_NAME use shared constants from core.h (kColType, kColName)
static constexpr int COL_TYPE = kColType; static constexpr int COL_TYPE = kColType;
static constexpr int COL_NAME = kColName; static constexpr int COL_NAME = kColName;
static constexpr int COL_VALUE = 22; static constexpr int COL_VALUE = kColValue;
static constexpr int COL_COMMENT = 28; // "// Enter=Save Esc=Cancel" fits static constexpr int COL_COMMENT = 28; // "// Enter=Save Esc=Cancel" fits
static const QString SEP = QStringLiteral(" "); static const QString SEP = QStringLiteral(" ");
@@ -36,22 +36,22 @@ QString typeName(NodeKind kind) {
// ── Value formatting ── // ── Value formatting ──
static QString hexStr(uint64_t v, int digits) { static QString hexVal(uint64_t v) {
return QStringLiteral("0x") + QString::number(v, 16).toUpper().rightJustified(digits, '0'); return QStringLiteral("0x") + QString::number(v, 16);
} }
static QString rawHex(uint64_t v, int digits) { static QString rawHex(uint64_t v, int digits) {
return QString::number(v, 16).toUpper().rightJustified(digits, '0'); return QString::number(v, 16).rightJustified(digits, '0');
} }
QString fmtInt8(int8_t v) { return QString::number(v); } QString fmtInt8(int8_t v) { return hexVal((uint8_t)v); }
QString fmtInt16(int16_t v) { return QString::number(v); } QString fmtInt16(int16_t v) { return hexVal((uint16_t)v); }
QString fmtInt32(int32_t v) { return QString::number(v); } QString fmtInt32(int32_t v) { return hexVal((uint32_t)v); }
QString fmtInt64(int64_t v) { return QString::number(v); } QString fmtInt64(int64_t v) { return hexVal((uint64_t)v); }
QString fmtUInt8(uint8_t v) { return hexStr(v, 2); } QString fmtUInt8(uint8_t v) { return hexVal(v); }
QString fmtUInt16(uint16_t v) { return hexStr(v, 4); } QString fmtUInt16(uint16_t v) { return hexVal(v); }
QString fmtUInt32(uint32_t v) { return hexStr(v, 8); } QString fmtUInt32(uint32_t v) { return hexVal(v); }
QString fmtUInt64(uint64_t v) { return hexStr(v, 16); } QString fmtUInt64(uint64_t v) { return hexVal(v); }
QString fmtFloat(float v) { return QString::number(v, 'f', 3); } QString fmtFloat(float v) { return QString::number(v, 'f', 3); }
QString fmtDouble(double v) { return QString::number(v, 'f', 6); } QString fmtDouble(double v) { return QString::number(v, 'f', 6); }
@@ -59,12 +59,12 @@ QString fmtBool(uint8_t v) { return v ? QStringLiteral("true") : QStringLiter
QString fmtPointer32(uint32_t v) { QString fmtPointer32(uint32_t v) {
if (v == 0) return QStringLiteral("-> NULL"); if (v == 0) return QStringLiteral("-> NULL");
return QStringLiteral("-> ") + hexStr(v, 8); return QStringLiteral("-> ") + hexVal(v);
} }
QString fmtPointer64(uint64_t v) { QString fmtPointer64(uint64_t v) {
if (v == 0) return QStringLiteral("-> NULL"); if (v == 0) return QStringLiteral("-> NULL");
return QStringLiteral("-> ") + hexStr(v, 16); return QStringLiteral("-> ") + hexVal(v);
} }
// ── Indentation ── // ── Indentation ──
@@ -137,10 +137,10 @@ static QString readValueImpl(const Node& node, const Provider& prov,
uint64_t addr, int subLine, ValueMode mode) { uint64_t addr, int subLine, ValueMode mode) {
const bool display = (mode == ValueMode::Display); const bool display = (mode == ValueMode::Display);
switch (node.kind) { switch (node.kind) {
case NodeKind::Hex8: return display ? hexStr(prov.readU8(addr), 2) : rawHex(prov.readU8(addr), 2); case NodeKind::Hex8: return display ? hexVal(prov.readU8(addr)) : rawHex(prov.readU8(addr), 2);
case NodeKind::Hex16: return display ? hexStr(prov.readU16(addr), 4) : rawHex(prov.readU16(addr), 4); case NodeKind::Hex16: return display ? hexVal(prov.readU16(addr)) : rawHex(prov.readU16(addr), 4);
case NodeKind::Hex32: return display ? hexStr(prov.readU32(addr), 8) : rawHex(prov.readU32(addr), 8); case NodeKind::Hex32: return display ? hexVal(prov.readU32(addr)) : rawHex(prov.readU32(addr), 8);
case NodeKind::Hex64: return display ? hexStr(prov.readU64(addr), 16): rawHex(prov.readU64(addr), 16); case NodeKind::Hex64: return display ? hexVal(prov.readU64(addr)) : rawHex(prov.readU64(addr), 16);
case NodeKind::Int8: return fmtInt8((int8_t)prov.readU8(addr)); case NodeKind::Int8: return fmtInt8((int8_t)prov.readU8(addr));
case NodeKind::Int16: return fmtInt16((int16_t)prov.readU16(addr)); case NodeKind::Int16: return fmtInt16((int16_t)prov.readU16(addr));
case NodeKind::Int32: return fmtInt32((int32_t)prov.readU32(addr)); case NodeKind::Int32: return fmtInt32((int32_t)prov.readU32(addr));
@@ -175,7 +175,7 @@ static QString readValueImpl(const Node& node, const Provider& prov,
line += QStringLiteral("]"); line += QStringLiteral("]");
return line; return line;
} }
case NodeKind::Padding: return display ? hexStr(prov.readU8(addr), 2) : rawHex(prov.readU8(addr), 2); case NodeKind::Padding: return display ? hexVal(prov.readU8(addr)) : rawHex(prov.readU8(addr), 2);
case NodeKind::UTF8: { case NodeKind::UTF8: {
QByteArray bytes = prov.readBytes(addr, node.strLen); QByteArray bytes = prov.readBytes(addr, node.strLen);
int end = bytes.indexOf('\0'); int end = bytes.indexOf('\0');
@@ -401,6 +401,34 @@ QString validateValue(NodeKind kind, const QString& text) {
QString s = text.trimmed(); QString s = text.trimmed();
if (s.isEmpty()) return {}; if (s.isEmpty()) return {};
// For integer/hex types, validate character set first
bool isHexKind = (kind >= NodeKind::Hex8 && kind <= NodeKind::Hex64)
|| kind == NodeKind::Pointer32 || kind == NodeKind::Pointer64;
bool isIntKind = (kind >= NodeKind::Int8 && kind <= NodeKind::UInt64);
if (isHexKind || isIntKind) {
bool hasHexPrefix = s.startsWith("0x", Qt::CaseInsensitive);
QString digits = hasHexPrefix ? s.mid(2) : s;
if (hasHexPrefix || isHexKind) {
// Hex mode: only 0-9, a-f, A-F
for (QChar c : digits) {
if (!c.isDigit() && !(c >= 'a' && c <= 'f') && !(c >= 'A' && c <= 'F'))
return QStringLiteral("invalid hex '%1'").arg(c);
}
} else {
// Decimal mode: only digits (and leading minus for signed)
int start = 0;
bool isSigned = (kind >= NodeKind::Int8 && kind <= NodeKind::Int64);
if (isSigned && !digits.isEmpty() && digits[0] == '-') start = 1;
for (int i = start; i < digits.size(); i++) {
if (!digits[i].isDigit())
return QStringLiteral("invalid '%1'").arg(digits[i]);
}
}
}
// Then do the actual parse for range checking
bool ok; bool ok;
parseValue(kind, text, &ok); parseValue(kind, text, &ok);
if (ok) return {}; if (ok) return {};
@@ -409,7 +437,7 @@ QString validateValue(NodeKind kind, const QString& text) {
const auto* m = kindMeta(kind); const auto* m = kindMeta(kind);
if (m && m->size > 0 && m->size <= 8) { if (m && m->size > 0 && m->size <= 8) {
uint64_t maxVal = (m->size == 8) ? ~0ULL : ((1ULL << (m->size * 8)) - 1); uint64_t maxVal = (m->size == 8) ? ~0ULL : ((1ULL << (m->size * 8)) - 1);
return QStringLiteral("0x%1 max").arg(maxVal, m->size * 2, 16, QChar('0')); return QStringLiteral("max 0x%1").arg(maxVal, m->size * 2, 16, QChar('0'));
} }
return QStringLiteral("invalid"); return QStringLiteral("invalid");
} }

View File

@@ -437,6 +437,93 @@ private slots:
QVERIFY(lm); QVERIFY(lm);
QVERIFY(sel.contains(lm->nodeIdx)); QVERIFY(sel.contains(lm->nodeIdx));
} }
// ── Test: value edit echoes to comment column ──
void testValueEditCommentEcho() {
m_editor->applyDocument(m_result);
// Begin value edit on line 1 (UInt16 field)
bool ok = m_editor->beginInlineEdit(EditTarget::Value, 1);
QVERIFY(ok);
QVERIFY(m_editor->isEditing());
// Get the line text before any typing
QString lineBefore;
int len = (int)m_editor->scintilla()->SendScintilla(
QsciScintillaBase::SCI_LINELENGTH, (unsigned long)1);
if (len > 0) {
QByteArray buf(len + 1, '\0');
m_editor->scintilla()->SendScintilla(
QsciScintillaBase::SCI_GETLINE, (unsigned long)1, (void*)buf.data());
lineBefore = QString::fromUtf8(buf.constData(), len).trimmed();
}
// Initial comment should contain "Enter=Save Esc=Cancel"
QVERIFY2(lineBefore.contains("Enter=Save"),
qPrintable("Initial comment missing, got: " + lineBefore));
// Type a digit to trigger validateEditLive
QKeyEvent key5(QEvent::KeyPress, Qt::Key_5, Qt::NoModifier, "5");
QApplication::sendEvent(m_editor->scintilla(), &key5);
QApplication::processEvents();
// Get line text after typing
QString lineAfter;
len = (int)m_editor->scintilla()->SendScintilla(
QsciScintillaBase::SCI_LINELENGTH, (unsigned long)1);
if (len > 0) {
QByteArray buf(len + 1, '\0');
m_editor->scintilla()->SendScintilla(
QsciScintillaBase::SCI_GETLINE, (unsigned long)1, (void*)buf.data());
lineAfter = QString::fromUtf8(buf.constData(), len).trimmed();
}
// Comment should show "!" prefix for invalid value
// Since "0x5a4d" + "5" = "0x5a4d5" = 370509 > 65535, it's invalid for UInt16
QVERIFY2(lineAfter.contains("! "),
qPrintable("Comment should show '!' for invalid value, got: " + lineAfter));
// Cancel and reset
m_editor->cancelInlineEdit();
m_editor->applyDocument(m_result);
}
// ── Test: value validation shows error indicator ──
void testValueValidationError() {
m_editor->applyDocument(m_result);
// Begin value edit on line 1 (UInt16 field, value = 23117)
bool ok = m_editor->beginInlineEdit(EditTarget::Value, 1);
QVERIFY(ok);
// Type "999" to make value invalid for UInt16 (appends to existing, making it too large)
// Original value 23117 -> typing "999" at end makes it invalid (23117999 > 65535)
const char* digits = "999";
for (int i = 0; digits[i]; i++) {
QKeyEvent key(QEvent::KeyPress, Qt::Key_9, Qt::NoModifier, QString(digits[i]));
QApplication::sendEvent(m_editor->scintilla(), &key);
QApplication::processEvents();
}
// Get line text - comment should show "! " prefix (error)
QString lineText;
int len = (int)m_editor->scintilla()->SendScintilla(
QsciScintillaBase::SCI_LINELENGTH, (unsigned long)1);
if (len > 0) {
QByteArray buf(len + 1, '\0');
m_editor->scintilla()->SendScintilla(
QsciScintillaBase::SCI_GETLINE, (unsigned long)1, (void*)buf.data());
lineText = QString::fromUtf8(buf.constData(), len).trimmed();
}
// Comment should show "! " prefix for invalid value
QVERIFY2(lineText.contains("! "),
qPrintable("Comment should show '! ' for invalid value, got: " + lineText));
// Cancel and reset
m_editor->cancelInlineEdit();
m_editor->applyDocument(m_result);
}
}; };
QTEST_MAIN(TestEditor) QTEST_MAIN(TestEditor)