Get rid of requestTrackFocus()

Author: juzzlin

src/application/service/editor_service.cpp

@@ -1288,10 +1288,10 @@ quint8 EditorService::delayAtCurrentPosition() const
 
 bool EditorService::requestPosition(const Position & position)
 {
-    return requestPosition(position.pattern, position.track, position.column, position.line, position.lineColumn);
+    return requestPosition(position.pattern, position.track, position.column, static_cast<qint32>(position.line), position.lineColumn);
 }
 
-bool EditorService::requestPosition(quint64 pattern, quint64 track, quint64 column, quint64 line, quint64 lineColumn)
+bool EditorService::requestPosition(quint64 pattern, quint64 track, quint64 column, qint64 line, quint64 lineColumn)
 {
     juzzlin::L(TAG).debug() << "Requesting position: " << pattern << " " << track << " " << column << " " << line << " " << lineColumn;
 
@@ -1300,10 +1300,10 @@ bool EditorService::requestPosition(quint64 pattern, quint64 track, quint64 colu
         return false;
     }
 
-    if (line >= m_song->lineCount(pattern)) {
-        juzzlin::L(TAG).error() << "Invalid line index: " << line;
-        return false;
-    }
+    // The requested line index can be outside the valid indices if clicked on the empty area of a column.
+    // In that case we'll just clamp to either zero or to the last index which is what the user very likely wants.
+    line = std::min(static_cast<int>(line), static_cast<int>(m_song->lineCount(pattern)) - 1);
+    line = std::max(static_cast<int>(line), 0);
 
     if (!m_song->hasTrack(track)) {
         juzzlin::L(TAG).error() << "Invalid track index: " << track;
@@ -1324,11 +1324,11 @@ bool EditorService::requestPosition(quint64 pattern, quint64 track, quint64 colu
     m_state.cursorPosition.pattern = pattern;
     m_state.cursorPosition.track = track;
     m_state.cursorPosition.column = column;
-    m_state.cursorPosition.line = line;
+    m_state.cursorPosition.line = static_cast<quint64>(line);
     m_state.cursorPosition.lineColumn = lineColumn;
     notifyPositionChange(oldPosition);
 
-    setCurrentTime(m_song->lineToTime(m_state.cursorPosition.line));
+    setCurrentTime(m_song->lineToTime(static_cast<quint32>(m_state.cursorPosition.line)));
 
     return true;
 }
@@ -1343,7 +1343,7 @@ void EditorService::requestPositionByTick(quint64 tick)
     const auto oldPosition = m_state.cursorPosition;
     if (auto && songPosition = m_song->songPositionByTick(tick); songPosition.has_value()) {
         m_state.cursorPosition.pattern = songPosition->pattern;
-        m_state.cursorPosition.line = songPosition->line;
+        m_state.cursorPosition.line = static_cast<quint64>(songPosition->line);
         notifyPositionChange(oldPosition);
         setSongPosition(songPosition->position);
         setCurrentTime(songPosition->currentTime);
@@ -1355,8 +1355,8 @@ void EditorService::requestScroll(int steps)
     const auto oldPosition = m_state.cursorPosition;
 
     // Work in signed domain to handle negative steps correctly
-    qint64 newLine = static_cast<qint64>(m_state.cursorPosition.line) + steps;
-    const qint64 lineCount = static_cast<qint64>(m_song->lineCount(m_state.cursorPosition.pattern));
+    auto newLine = static_cast<qint64>(m_state.cursorPosition.line) + steps;
+    const auto lineCount = static_cast<qint64>(m_song->lineCount(m_state.cursorPosition.pattern));
 
     // Wrap around correctly
     if (newLine < 0) {
@@ -1371,25 +1371,6 @@ void EditorService::requestScroll(int steps)
     notifyPositionChange(oldPosition);
 }
 
-void EditorService::requestTrackFocus(quint64 trackIndex, quint64 column, int line)
-{
-    if (m_song->hasTrack(trackIndex)) {
-        juzzlin::L(TAG).info() << "Focus for track " << trackIndex << " on column " << column << " on line " << line << " requested";
-        if (column < m_song->columnCount(trackIndex)) {
-            // The requested line index can be outside the valid indices if clicked on the empty area of a column.
-            // In that case we'll just clamp to either zero or to the last index which is what the user very likely wants.
-            line = std::min(line, static_cast<int>(currentLineCount()) - 1);
-            line = std::max(line, 0);
-            const auto oldPosition = m_state.cursorPosition;
-            m_state.cursorPosition.track = trackIndex;
-            m_state.cursorPosition.column = column;
-            m_state.cursorPosition.line = static_cast<quint64>(line);
-            m_state.cursorPosition.lineColumn = 0;
-            notifyPositionChange(oldPosition);
-        }
-    }
-}
-
 quint64 EditorService::ticksPerLine() const
 {
     return m_song->ticksPerLine();

src/application/service/editor_service.hpp

@@ -199,9 +199,8 @@ class EditorService : public QObject
     Q_INVOKABLE quint8 delayAtCurrentPosition() const;
 
     bool requestPosition(const Position & position);
-    Q_INVOKABLE bool requestPosition(quint64 pattern, quint64 track, quint64 column, quint64 line, quint64 lineColumn);
+    Q_INVOKABLE bool requestPosition(quint64 pattern, quint64 track, quint64 column, qint64 line, quint64 lineColumn);
     Q_INVOKABLE void requestScroll(int steps);
-    Q_INVOKABLE void requestTrackFocus(quint64 trackIndex, quint64 column, int line);
 
     Q_INVOKABLE quint64 ticksPerLine() const;
     Q_INVOKABLE quint64 beatsPerMinute() const;

src/unit_tests/editor_service_test/editor_service_test.cpp

@@ -994,7 +994,6 @@ void EditorServiceTest::test_requestPosition_invalidPosition_shouldNotChangePosi
     QVERIFY(!editorService.requestPosition(neg, 0, 0, 0, 0));
     QVERIFY(!editorService.requestPosition(0, neg, 0, 0, 0));
     QVERIFY(!editorService.requestPosition(0, 0, neg, 0, 0));
-    QVERIFY(!editorService.requestPosition(0, 0, 0, neg, 0));
     QVERIFY(!editorService.requestPosition(0, 0, 0, 0, neg));
     QCOMPARE(positionChangedSpy.count(), 0);
     QCOMPARE(currentTimeChangedSpy.count(), 0);
@@ -1079,35 +1078,35 @@ void EditorServiceTest::test_requestScroll_shouldChangeCurrentTime()
     QCOMPARE(editorService.currentTime(), "00:00:00.062");
 }
 
-void EditorServiceTest::test_requestTrackFocus_shouldChangePosition()
+void EditorServiceTest::test_requestPosition_shouldChangePosition()
 {
     EditorService editorService;
     QSignalSpy positionChangedSpy { &editorService, &EditorService::positionChanged };
 
-    editorService.requestTrackFocus(0, 0, 0);
+    editorService.requestPosition(0, 0, 0, 0, 0);
     QCOMPARE(positionChangedSpy.count(), 1);
 
-    editorService.requestTrackFocus(0, 1, 0);
+    editorService.requestPosition(0, 0, 1, 0, 0);
     QCOMPARE(positionChangedSpy.count(), 1);
 
     editorService.requestNewColumn(0);
     QCOMPARE(positionChangedSpy.count(), 2);
-    editorService.requestTrackFocus(0, 1, 0);
+    editorService.requestPosition(0, 0, 1, 0, 0);
     QCOMPARE(positionChangedSpy.count(), 3);
     QCOMPARE(editorService.position().track, 0);
     QCOMPARE(editorService.position().column, 1);
 
-    editorService.requestTrackFocus(editorService.trackCount() - 1, 0, 0);
+    editorService.requestPosition(0, editorService.trackCount() - 1, 0, 0, 0);
     QCOMPARE(positionChangedSpy.count(), 4);
     QCOMPARE(editorService.position().track, editorService.trackCount() - 1);
 }
 
-void EditorServiceTest::test_requestTrackFocus_shouldNotChangePosition()
+void EditorServiceTest::test_requestPosition_shouldNotChangePosition()
 {
     EditorService editorService;
     QSignalSpy positionChangedSpy { &editorService, &EditorService::positionChanged };
 
-    editorService.requestTrackFocus(editorService.trackCount(), 0, 0);
+    editorService.requestPosition(0, editorService.trackCount(), 0, 0, 0);
 
     QCOMPARE(positionChangedSpy.count(), 0);
     QCOMPARE(editorService.position().track, 0);

src/unit_tests/editor_service_test/editor_service_test.hpp

@@ -82,14 +82,13 @@ private slots:
 
     void test_requestPosition_invalidPosition_shouldNotChangePosition();
     void test_requestPosition_validPosition_shouldChangePosition();
+    void test_requestPosition_shouldChangePosition();
+    void test_requestPosition_shouldNotChangePosition();
     void test_resetSongPosition_firstTrackRemoved_shouldResetPosition();
 
     void test_requestScroll_shouldChangePosition();
     void test_requestScroll_shouldChangeCurrentTime();
 
-    void test_requestTrackFocus_shouldChangePosition();
-    void test_requestTrackFocus_shouldNotChangePosition();
-
     void test_setCurrentLineCount_shouldSetLineCount();
 
     void test_setCurrentPattern_shouldCreatePattern();

src/view/qml/Editor/MouseHandler.qml

@@ -9,11 +9,11 @@ QtObject {
     property int _selectionEndLine: 0
     property bool _isDragging: false
     function handleLeftClicked(track: var, columnIndex: int, lineIndex: int, x: int, y: int): void {
-        editorService.requestTrackFocus(track.index(), columnIndex, lineIndex);
+        editorService.requestPosition(track.patternIndex(), track.index(), columnIndex, lineIndex, 0);
         editorFocusRequested();
     }
     function handleRightClicked(track: var, columnIndex: int, lineIndex: int, x: int, y: int): void {
-        editorService.requestTrackFocus(track.index(), columnIndex, lineIndex);
+        editorService.requestPosition(track.patternIndex(), track.index(), columnIndex, lineIndex, 0);
         editorFocusRequested();
         UiService.requestContextMenu(x, y);
     }

src/view/qml/Editor/Track.qml

@@ -23,6 +23,9 @@ Rectangle {
     function index(): int {
         return _index;
     }
+    function patternIndex(): int {
+        return _patternIndex;
+    }
     function setLocation(patternIndex: int, trackIndex: int): void {
         _patternIndex = patternIndex;
         _index = trackIndex;