From 6dd8b34eb5fba99c7b60dffdd2f98e87418a1306 Mon Sep 17 00:00:00 2001 From: Thomas Zander Date: Fri, 28 Mar 2008 18:21:43 +0100 Subject: [PATCH] Refactor the cursor interface to have an 'invalid' state so we can better test if the interaction is done --- Interview.cpp | 23 +++--- InterviewCursor.h | 29 +++++++- hunks/HunksCursor.cpp | 57 ++++++++------- hunks/HunksCursor.h | 7 +- hunks/tests/TestCursor.cpp | 176 ++++++++++++++++++++++++++++++++++++++++++--- hunks/tests/TestCursor.h | 1 + 6 files changed, 240 insertions(+), 53 deletions(-) diff --git a/Interview.cpp b/Interview.cpp index 59616fe..20121ad 100644 --- a/Interview.cpp +++ b/Interview.cpp @@ -49,43 +49,41 @@ bool Interview::start() patchOut.flush(); Logger::stopPager(); - int i = currentIndex; bool askAgain = false; do { int count = m_cursor.count(); QString allowed = "ynsfqadjk"; if (count == -1) allowed += "c"; - QString question = prompt +" ("+ QString::number(i) +"/"+ + QString question = prompt +" ("+ QString::number(currentIndex) +"/"+ (count == -1 ? "?" : QString::number(count)) +") ["+ allowed + "], or ? for help: "; QChar reply = askSingleChar(question); askAgain = false; switch (reply.toAscii()) { - case 'j': i = m_cursor.forward(InterviewCursor::ItemScope, false); break; + case 'j': currentIndex = m_cursor.forward(InterviewCursor::ItemScope, false); break; case 'k': if (currentIndex == 1) // at start. askAgain = true; else - i = m_cursor.back(); + currentIndex = m_cursor.back(); break; case 'y': m_cursor.setResponse(true); - i = m_cursor.forward(); break; + currentIndex = m_cursor.forward(); break; case 'n': m_cursor.setResponse(false); - i = m_cursor.forward(); break; + currentIndex = m_cursor.forward(); break; case 'f': m_cursor.setResponse(true, InterviewCursor::BlockScope); - i = m_cursor.forward(InterviewCursor::BlockScope); break; + currentIndex = m_cursor.forward(InterviewCursor::BlockScope); break; case 's': m_cursor.setResponse(false, InterviewCursor::BlockScope); - i = m_cursor.forward(InterviewCursor::BlockScope); break; + currentIndex = m_cursor.forward(InterviewCursor::BlockScope); break; case 'a': m_cursor.setResponse(true, InterviewCursor::FullRange); return true; case 'd': return true; - break; - case 'q': - return false; + case 'q': return false; case 'h': // fall through case '?': out << m_cursor.helpMessage(); + out.flush(); askAgain = true; break; case 'c': @@ -98,9 +96,8 @@ bool Interview::start() break; } } while(askAgain); - if (i == currentIndex) + if (! m_cursor.isValid()) return true; - currentIndex = i; } } diff --git a/InterviewCursor.h b/InterviewCursor.h index 6a3fb23..ee103a6 100644 --- a/InterviewCursor.h +++ b/InterviewCursor.h @@ -38,15 +38,40 @@ public: m_config = &config; } - /// returns the new index + /** + * Moves the cursor forward in the list. + * returns the new index. + * Moving the cursor past the last item will make isValid() return false. + */ virtual int forward(Scope scope = ItemScope, bool skipAnswered = true) = 0; - /// returns the new index + + /** + * Moves the cursor backwards in the list. + * returns the new index. + * This method can not move the cursor to be before the first item. + */ virtual int back(Scope scope = ItemScope) = 0; virtual void setResponse(bool response, Scope scope = ItemScope) = 0; virtual QString currentText() const = 0; + /** + * Returns true if the item the cursor is on is a valid one. The cursor can point to an invalid item when there + * are no items in the document, or when the cursor moved past the end of the list. + */ + virtual bool isValid() const = 0; + + /** + * Returns the amount of items in the list, or -1 if the amount is not fully determined yet. + * @see forceCount() + */ virtual int count() = 0; + + /** + * Will force a count of all items, potentially blocking for extended times while counting. + * Afterwards the count() method will return a valid count. + */ virtual void forceCount() = 0; + virtual QString helpMessage() const = 0; protected: diff --git a/hunks/HunksCursor.cpp b/hunks/HunksCursor.cpp index d08ba09..63dc210 100644 --- a/hunks/HunksCursor.cpp +++ b/hunks/HunksCursor.cpp @@ -22,9 +22,9 @@ HunksCursor::HunksCursor(ChangeSet &changeSet) : m_changeSet(changeSet), + m_totalHunks(-1), m_fileIndex(0), m_hunkIndex(0), - m_totalHunks(-1), m_subHunkIndex(0) { if (changeSet.count()) { @@ -45,9 +45,8 @@ HunksCursor::~HunksCursor() int HunksCursor::forward(Scope scope, bool skipAnswered) { - if (m_changeSet.count() == 0) + if (m_changeSet.count() == 0 || m_fileIndex >= m_changeSet.count()) return currentIndex(); - Q_ASSERT(m_fileIndex < m_changeSet.count()); struct Last { Last(const ChangeSet &changeSet) @@ -65,8 +64,12 @@ int HunksCursor::forward(Scope scope, bool skipAnswered) int file, hunk, subHunk; }; Last last(m_changeSet); - if (m_fileIndex >= last.file && m_hunkIndex >= last.hunk && m_subHunkIndex >= last.subHunk) + if (m_fileIndex > last.file) { + m_fileIndex = last.file +1; + m_hunkIndex = 0; + m_subHunkIndex = 0; return currentIndex(); // already at end. + } switch(scope) { case ItemScope: { @@ -75,11 +78,8 @@ int HunksCursor::forward(Scope scope, bool skipAnswered) --m_subHunkIndex; File file = m_changeSet.files().at(m_fileIndex); - if (m_hunkIndex + 1 - 2 >= file.count()) { - if (m_fileIndex == m_changeSet.count() -1) - return currentIndex(); // reached the end. + if (m_hunkIndex + 1 - 2 >= file.count()) return forward(BlockScope, skipAnswered); - } else { m_hunkIndex++; m_subHunkIndex = 0; @@ -87,24 +87,22 @@ int HunksCursor::forward(Scope scope, bool skipAnswered) break; } case BlockScope: - if (m_fileIndex < last.file) { - m_fileIndex++; - m_hunkIndex = 0; - m_subHunkIndex = 0; - } - else { + m_fileIndex++; + m_hunkIndex = 0; + m_subHunkIndex = 0; + if (m_fileIndex > last.file) skipAnswered = false; // there is nothing left; - m_hunkIndex = last.hunk; - m_subHunkIndex = last.subHunk; - } break; case FullRange: - m_fileIndex = last.file; - m_hunkIndex = last.hunk; - m_subHunkIndex = last.subHunk; + m_fileIndex = last.file + 1; + m_hunkIndex = 0; + m_subHunkIndex = 0; break; } + if (! isValid()) + return currentIndex(); + File file = m_changeSet.files()[m_fileIndex]; const bool renamed = file.fileName() != file.oldFileName(); const bool protectionChanged = !renamed && file.protection() != file.oldProtection(); @@ -143,9 +141,9 @@ int HunksCursor::back(Scope scope) ++m_subHunkIndex; if (--m_hunkIndex < 1) { m_hunkIndex++; - File file = m_changeSet.files()[m_fileIndex]; return back(BlockScope); } + m_subHunkIndex = currentSubHunkCount() - 1; break; case BlockScope: if (--m_fileIndex < 0) @@ -234,7 +232,7 @@ void HunksCursor::forceCount() int HunksCursor::currentIndex() const { - int answer = m_subHunkIndex; + int answer = 1; QList files = m_changeSet.files().mid(0, m_fileIndex + 1); QList::Iterator iter = files.begin(); while(iter != files.end()) { @@ -243,13 +241,11 @@ int HunksCursor::currentIndex() const const bool protectionChanged = !renamed && file.protection() != file.oldProtection(); QList hunks = file.hunks(); ++iter; - if (iter == files.end()) { + if (iter == files.end() && m_fileIndex < m_changeSet.count()) { if (m_hunkIndex > 0 && renamed) answer++; if (m_hunkIndex > 1 && protectionChanged) answer++; - if (m_hunkIndex < 2) - return answer + 1; hunks = hunks.mid(0, m_hunkIndex - 2); } else { @@ -259,12 +255,15 @@ int HunksCursor::currentIndex() const answer++; } QList::Iterator iter2 = hunks.begin(); + int index = 2; while(iter2 != hunks.end()) { + if (isValid() && iter == files.end() && m_hunkIndex < index++) + break; answer += (*iter2).subHunkCount(); ++iter2; } } - return answer + 1; + return answer + m_subHunkIndex; } QString HunksCursor::currentText() const @@ -358,3 +357,9 @@ int HunksCursor::currentSubHunkCount() Hunk hunk = file.hunks()[m_hunkIndex - 2]; return hunk.subHunkCount(); } + +bool HunksCursor::isValid() const +{ + return m_changeSet.count() > 0 && m_changeSet.count() > m_fileIndex; +} + diff --git a/hunks/HunksCursor.h b/hunks/HunksCursor.h index ff2c6c3..16964f1 100644 --- a/hunks/HunksCursor.h +++ b/hunks/HunksCursor.h @@ -38,15 +38,18 @@ public: virtual void forceCount(); virtual QString currentText() const; virtual QString helpMessage() const; + virtual bool isValid() const; private: - int currentIndex() const; int currentSubHunkCount(); ChangeSet &m_changeSet; + int m_totalHunks; + +protected: // unit test. + int currentIndex() const; int m_fileIndex; int m_hunkIndex; - int m_totalHunks; int m_subHunkIndex; }; diff --git a/hunks/tests/TestCursor.cpp b/hunks/tests/TestCursor.cpp index bdf589d..702f4c1 100644 --- a/hunks/tests/TestCursor.cpp +++ b/hunks/tests/TestCursor.cpp @@ -1,6 +1,40 @@ #include "TestCursor.h" #include "../HunksCursor.h" +class MyHunksCursor : public HunksCursor { +public: + MyHunksCursor(ChangeSet &changeSet) + : HunksCursor(changeSet) + { + } + + int atFile() const + { + return m_fileIndex; + } + int atHunk() const + { + return m_hunkIndex; + } + + int atSubhunk() const + { + return m_subHunkIndex; + } + + void setPosition(int file, int hunk, int subhunk) + { + m_fileIndex = file; + m_hunkIndex = hunk; + m_subHunkIndex = subhunk; + } + + int currentIndexOpen() + { + return currentIndex(); + } +}; + void TestCursor::testBasis() { // these are all using the same backend to make moving around objects easier. File file1; @@ -27,6 +61,7 @@ void TestCursor::testBasis() { QCOMPARE(hunk3.header(), fn); } + void TestCursor::testForward1() { ChangeSet changeSet; File file; @@ -38,12 +73,23 @@ void TestCursor::testForward1() { QCOMPARE(changeSet.count(), 1); - HunksCursor cursor(changeSet); - QCOMPARE(cursor.currentText(), QString("move `bar' `foo'\n")); - QCOMPARE(cursor.forward(), 1); + MyHunksCursor cursor(changeSet); + QCOMPARE(cursor.isValid(), true); QCOMPARE(cursor.currentText(), QString("move `bar' `foo'\n")); + QCOMPARE(cursor.forward(), 2); + QCOMPARE(cursor.atFile(), 1); + QCOMPARE(cursor.atHunk(), 0); + QCOMPARE(cursor.atSubhunk(), 0); + QCOMPARE(cursor.isValid(), false); + QCOMPARE(cursor.currentText(), QString()); + QCOMPARE(cursor.back(), 1); + QCOMPARE(cursor.atFile(), 0); + QCOMPARE(cursor.atHunk(), 0); + QCOMPARE(cursor.atSubhunk(), 0); + QCOMPARE(cursor.isValid(), true); QCOMPARE(cursor.back(), 1); + QCOMPARE(cursor.isValid(), true); } void TestCursor::testForward2() { @@ -58,8 +104,8 @@ void TestCursor::testForward2() { File file; file.setFileName("filename"); file.setOldFileName("filename"); - newFile.setProtection("100644"); - newFile.setOldProtection("100644"); + file.setProtection("100644"); + file.setOldProtection("100644"); Hunk hunk; hunk.addLine(QByteArray("@@ -1,4 +1,4 @@")); hunk.addLine(" foo"); @@ -76,20 +122,36 @@ void TestCursor::testForward2() { QCOMPARE(changeSet.count(), 2); - HunksCursor cursor(changeSet); + MyHunksCursor cursor(changeSet); cursor.forceCount(); + QCOMPARE(cursor.isValid(), true); QCOMPARE(cursor.count(), 3); QCOMPARE(cursor.back(), 1); + QCOMPARE(cursor.atFile(), 0); + QCOMPARE(cursor.atHunk(), 0); + QCOMPARE(cursor.atSubhunk(), 0); QCOMPARE(cursor.forward(), 2); + QCOMPARE(cursor.atFile(), 1); + QCOMPARE(cursor.atHunk(), 2); + QCOMPARE(cursor.atSubhunk(), 0); + QCOMPARE(cursor.isValid(), true); QCOMPARE(cursor.forward(), 3); - QCOMPARE(cursor.forward(), 3); + QCOMPARE(cursor.atFile(), 1); + QCOMPARE(cursor.atHunk(), 2); + QCOMPARE(cursor.atSubhunk(), 1); + QCOMPARE(cursor.isValid(), true); + QCOMPARE(cursor.forward(), 4); + QCOMPARE(cursor.atFile(), 2); // non existing file. + QCOMPARE(cursor.isValid(), false); QCOMPARE(cursor.back(InterviewCursor::FullRange), 1); - QCOMPARE(cursor.forward(InterviewCursor::FullRange), 3); + QCOMPARE(cursor.isValid(), true); + QCOMPARE(cursor.forward(InterviewCursor::FullRange), 4); + QCOMPARE(cursor.isValid(), false); QCOMPARE(cursor.back(InterviewCursor::FullRange), 1); QCOMPARE(cursor.forward(InterviewCursor::BlockScope), 2); - QCOMPARE(cursor.forward(InterviewCursor::BlockScope), 3); + QCOMPARE(cursor.forward(InterviewCursor::BlockScope), 4); QCOMPARE(cursor.back(InterviewCursor::FullRange), 1); cursor.setResponse(true); @@ -103,7 +165,101 @@ void TestCursor::testForward2() { QCOMPARE(cursor.back(InterviewCursor::FullRange), 1); QCOMPARE(cursor.forward(InterviewCursor::ItemScope, false), 2); QCOMPARE(cursor.forward(InterviewCursor::ItemScope, false), 3); - QCOMPARE(cursor.forward(InterviewCursor::ItemScope, true), 3); + QCOMPARE(cursor.forward(InterviewCursor::ItemScope, true), 4); + + File file2; + file2.setFileName("document"); + file2.setOldFileName("document"); + file2.setProtection("100644"); + file2.setOldProtection("100644"); + hunk = Hunk(); + hunk.addLine(QByteArray("@@ -10,4 +10,4 @@")); + hunk.addLine(" foo"); + hunk.addLine("+bar"); + hunk.addLine(" separator"); + hunk.addLine("-another subhunk"); + file2.addHunk(hunk); + ChangeSet cs; + cs.addFile(newFile); + cs.addFile(file); + cs.addFile(file2); + MyHunksCursor myCursor(cs); + + QCOMPARE(myCursor.atFile(), 0); + QCOMPARE(myCursor.atHunk(), 0); + QCOMPARE(myCursor.atSubhunk(), 0); + QCOMPARE(myCursor.back(InterviewCursor::FullRange), 1); + QCOMPARE(myCursor.atFile(), 0); + QCOMPARE(myCursor.atHunk(), 0); + QCOMPARE(myCursor.atSubhunk(), 0); + QCOMPARE(myCursor.forward(InterviewCursor::ItemScope, false), 2); + QCOMPARE(myCursor.atFile(), 1); + QCOMPARE(myCursor.atHunk(), 2); + QCOMPARE(myCursor.atSubhunk(), 0); + QCOMPARE(myCursor.forward(InterviewCursor::ItemScope, false), 3); + QCOMPARE(myCursor.atFile(), 1); + QCOMPARE(myCursor.atHunk(), 2); + QCOMPARE(myCursor.atSubhunk(), 1); + QCOMPARE(myCursor.forward(InterviewCursor::ItemScope, false), 4); + QCOMPARE(myCursor.atFile(), 2); + QCOMPARE(myCursor.atHunk(), 2); + QCOMPARE(myCursor.atSubhunk(), 0); + QCOMPARE(myCursor.forward(InterviewCursor::ItemScope, false), 5); + QCOMPARE(myCursor.atFile(), 2); + QCOMPARE(myCursor.atHunk(), 2); + QCOMPARE(myCursor.atSubhunk(), 1); + QCOMPARE(myCursor.forward(InterviewCursor::ItemScope, false), 6); + QCOMPARE(myCursor.atFile(), 3); + QCOMPARE(myCursor.atHunk(), 0); + QCOMPARE(myCursor.atSubhunk(), 0); + + QCOMPARE(myCursor.back(InterviewCursor::ItemScope), 5); + QCOMPARE(myCursor.back(InterviewCursor::ItemScope), 4); + QCOMPARE(myCursor.back(InterviewCursor::ItemScope), 3); + QCOMPARE(myCursor.back(InterviewCursor::ItemScope), 2); + QCOMPARE(myCursor.back(InterviewCursor::ItemScope), 1); + QCOMPARE(myCursor.back(InterviewCursor::ItemScope), 1); +} + +void TestCursor::testBackwards() +{ + ChangeSet changeSet; + File file; + file.setFileName("filename"); + file.setOldFileName("filename"); + file.setProtection("100644"); + file.setOldProtection("100644"); + QList patch; + patch.append(" foo\n"); + patch.append("+bar\n"); + patch.append("+b\n"); + patch.append("+c\n"); + patch.append(" separator\n"); + patch.append("-another subhunk\n"); + patch.append("-c\n"); + patch.append(" separator\n"); + patch.append("+another subhunk\n"); + + Hunk hunk; + hunk.addLine(QByteArray("@@ -1,4 +1,4 @@")); + foreach(QByteArray s, patch) hunk.addLine(s); + file.addHunk(hunk); + + Hunk hunk2; + hunk2.addLine(QByteArray("@@ -30,4 +40,4 @@")); + foreach(QByteArray s, patch) hunk2.addLine(s); + file.addHunk(hunk2); + changeSet.addFile(file); + + MyHunksCursor myCursor(changeSet); + myCursor.setPosition(0, 3, 2); + QCOMPARE(myCursor.currentIndexOpen(), 6); + QCOMPARE(myCursor.back(InterviewCursor::ItemScope), 5); + QCOMPARE(myCursor.back(InterviewCursor::ItemScope), 4); + QCOMPARE(myCursor.back(InterviewCursor::ItemScope), 3); + QCOMPARE(myCursor.back(InterviewCursor::ItemScope), 2); + QCOMPARE(myCursor.back(InterviewCursor::ItemScope), 1); + QCOMPARE(myCursor.back(InterviewCursor::ItemScope), 1); } QTEST_MAIN(TestCursor) diff --git a/hunks/tests/TestCursor.h b/hunks/tests/TestCursor.h index f51c384..0de6e6d 100644 --- a/hunks/tests/TestCursor.h +++ b/hunks/tests/TestCursor.h @@ -13,6 +13,7 @@ private slots: void testBasis(); void testForward1(); void testForward2(); + void testBackwards(); }; #endif -- 2.11.4.GIT