fix: Correctly pick whole file context (#85)

Currently the current line is duplicated in both "before" and "after"
context. This is due to DocumentContextReader::readWholeFileAfter()
picking the current line part of which has been already included into
the "before" context.

Author: p12tic

context/DocumentContextReader.cpp

@@ -91,14 +91,14 @@ QString DocumentContextReader::getContextBefore(
         effectiveStartLine = qMax(0, lineNumber - linesCount);
     }
 
-    return getContextBetween(effectiveStartLine, lineNumber, cursorPosition);
+    return getContextBetween(effectiveStartLine, -1, lineNumber, cursorPosition);
 }
 
 QString DocumentContextReader::getContextAfter(
     int lineNumber, int cursorPosition, int linesCount) const
 {
     int endLine = qMin(m_document->blockCount() - 1, lineNumber + linesCount);
-    return getContextBetween(lineNumber + 1, endLine, cursorPosition);
+    return getContextBetween(lineNumber + 1, cursorPosition, endLine, -1);
 }
 
 QString DocumentContextReader::readWholeFileBefore(int lineNumber, int cursorPosition) const
@@ -110,14 +110,12 @@ QString DocumentContextReader::readWholeFileBefore(int lineNumber, int cursorPos
 
     startLine = qMin(startLine, lineNumber);
 
-    QString result = getContextBetween(startLine, lineNumber, cursorPosition);
-
-    return result;
+    return getContextBetween(startLine, -1, lineNumber, cursorPosition);
 }
 
 QString DocumentContextReader::readWholeFileAfter(int lineNumber, int cursorPosition) const
 {
-    return getContextBetween(lineNumber, m_document->blockCount() - 1, cursorPosition);
+    return getContextBetween(lineNumber, cursorPosition, m_document->blockCount() - 1, -1);
 }
 
 QString DocumentContextReader::getLanguageAndFileInfo() const
@@ -172,18 +170,71 @@ CopyrightInfo DocumentContextReader::findCopyright()
     return result;
 }
 
-QString DocumentContextReader::getContextBetween(int startLine, int endLine, int cursorPosition) const
+QString DocumentContextReader::getContextBetween(
+    int startLine, int startCursorPosition, int endLine, int endCursorPosition) const
 {
     QString context;
-    for (int i = startLine; i <= endLine; ++i) {
-        QTextBlock block = m_document->findBlockByNumber(i);
+
+    if (startLine > endLine) {
+        return context;
+    }
+
+    if (startLine == endLine) {
+        auto block = m_document->findBlockByNumber(startLine);
+        if (!block.isValid()) {
+            return context;
+        }
+
+        auto text = block.text();
+
+        if (startCursorPosition < 0) {
+            startCursorPosition = 0;
+        }
+        if (endCursorPosition < 0) {
+            endCursorPosition = text.size();
+        }
+
+        if (startCursorPosition >= endCursorPosition) {
+            return context;
+        }
+
+        return text.mid(startCursorPosition, endCursorPosition - startCursorPosition);
+    }
+
+    // first line
+    {
+        auto block = m_document->findBlockByNumber(startLine);
+        if (!block.isValid()) {
+            return context;
+        }
+        auto text = block.text();
+        if (startCursorPosition < 0) {
+            context += text + "\n";
+        } else {
+            context += text.right(text.size() - startCursorPosition) + "\n";
+        }
+    }
+
+    // intermediate lines, if any
+    for (int i = startLine + 1; i <= endLine - 1; ++i) {
+        auto block = m_document->findBlockByNumber(i);
+        if (!block.isValid()) {
+            return context;
+        }
+        context += block.text() + "\n";
+    }
+
+    // last line
+    {
+        auto block = m_document->findBlockByNumber(endLine);
         if (!block.isValid()) {
-            break;
+            return context;
         }
-        if (i == endLine) {
-            context += block.text().left(cursorPosition);
+        auto text = block.text();
+        if (endCursorPosition < 0) {
+            context += text;
         } else {
-            context += block.text() + "\n";
+            context += text.left(endCursorPosition);
         }
     }
 
@@ -222,7 +273,7 @@ QString DocumentContextReader::getContextBefore(int lineNumber, int cursorPositi
         } else {
             effectiveStartLine = qMax(0, lineNumber - beforeCursor);
         }
-        return getContextBetween(effectiveStartLine, lineNumber, cursorPosition);
+        return getContextBetween(effectiveStartLine, -1, lineNumber, cursorPosition);
     }
 }
 
@@ -234,7 +285,7 @@ QString DocumentContextReader::getContextAfter(int lineNumber, int cursorPositio
         int endLine = qMin(
             m_document->blockCount() - 1,
             lineNumber + Settings::codeCompletionSettings().readStringsAfterCursor());
-        return getContextBetween(lineNumber + 1, endLine, -1);
+        return getContextBetween(lineNumber + 1, cursorPosition, endLine, -1);
     }
 }
 

context/DocumentContextReader.hpp

@@ -46,7 +46,8 @@ class DocumentContextReader
     QString readWholeFileAfter(int lineNumber, int cursorPosition) const;
     QString getLanguageAndFileInfo() const;
     CopyrightInfo findCopyright();
-    QString getContextBetween(int startLine, int endLine, int cursorPosition) const;
+    QString getContextBetween(
+        int startLine, int startCursorPosition, int endLine, int endCursorPosition) const;
 
     CopyrightInfo copyrightInfo() const;
 

test/DocumentContextReaderTest.cpp

@@ -93,7 +93,7 @@ TEST_F(DocumentContextReaderTest, testGetContextAfterWithCopyright)
     EXPECT_EQ(reader.getContextAfter(0, -1, 2), "Line 1\nLine 2");
 
     // Test getting context after with copyright skipped
-    EXPECT_EQ(reader.getContextAfter(1, 0, 2), "Line 2\n");
+    EXPECT_EQ(reader.getContextAfter(1, 0, 2), "Line 2\nLine 3");
 }
 
 TEST_F(DocumentContextReaderTest, testReadWholeFile)
@@ -112,7 +112,9 @@ TEST_F(DocumentContextReaderTest, testReadWholeFileWithCopyright)
     EXPECT_EQ(reader.readWholeFileAfter(2, -1), "Line 2\nLine 3\nLine 4");
 
     EXPECT_EQ(reader.readWholeFileBefore(2, 0), "Line 1\n");
-    EXPECT_EQ(reader.readWholeFileAfter(2, 0), "Line 2\nLine 3\n");
+    EXPECT_EQ(reader.readWholeFileAfter(2, 0), "Line 2\nLine 3\nLine 4");
+    EXPECT_EQ(reader.readWholeFileBefore(2, 2), "Line 1\nLi");
+    EXPECT_EQ(reader.readWholeFileAfter(2, 2), "ne 2\nLine 3\nLine 4");
 }
 
 TEST_F(DocumentContextReaderTest, testReadWholeFileWithMultilineCopyright)
@@ -124,8 +126,10 @@ TEST_F(DocumentContextReaderTest, testReadWholeFileWithMultilineCopyright)
     EXPECT_EQ(reader.readWholeFileBefore(6, -1), "Line 1\nLine 2");
     EXPECT_EQ(reader.readWholeFileAfter(5, -1), "Line 1\nLine 2");
 
-    EXPECT_EQ(reader.readWholeFileBefore(6, 4), "Line 1\nLine");
-    EXPECT_EQ(reader.readWholeFileAfter(5, 4), "Line 1\nLine");
+    EXPECT_EQ(reader.readWholeFileBefore(6, 0), "Line 1\n");
+    EXPECT_EQ(reader.readWholeFileAfter(6, 0), "Line 2");
+    EXPECT_EQ(reader.readWholeFileBefore(6, 2), "Line 1\nLi");
+    EXPECT_EQ(reader.readWholeFileAfter(6, 2), "ne 2");
 }
 
 TEST_F(DocumentContextReaderTest, testFindCopyrightSingleLine)
@@ -173,8 +177,17 @@ TEST_F(DocumentContextReaderTest, testGetContextBetween)
 {
     auto reader = createTestReader("Line 1\nLine 2\nLine 3\nLine 4\nLine 5");
 
-    EXPECT_EQ(reader.getContextBetween(1, 3, -1), "Line 2\nLine 3\nLine 4");
-    EXPECT_EQ(reader.getContextBetween(0, 2, 4), "Line 1\nLine 2\nLine");
+    EXPECT_EQ(reader.getContextBetween(2, -1, 0, -1), "");
+    EXPECT_EQ(reader.getContextBetween(0, -1, 0, -1), "Line 1");
+    EXPECT_EQ(reader.getContextBetween(1, -1, 1, -1), "Line 2");
+    EXPECT_EQ(reader.getContextBetween(1, 3, 1, 1), "");
+    EXPECT_EQ(reader.getContextBetween(1, 3, 1, 3), "");
+    EXPECT_EQ(reader.getContextBetween(1, 3, 1, 4), "e");
+
+    EXPECT_EQ(reader.getContextBetween(1, -1, 3, -1), "Line 2\nLine 3\nLine 4");
+    EXPECT_EQ(reader.getContextBetween(1, 2, 3, -1), "ne 2\nLine 3\nLine 4");
+    EXPECT_EQ(reader.getContextBetween(1, -1, 3, 2), "Line 2\nLine 3\nLi");
+    EXPECT_EQ(reader.getContextBetween(1, 2, 3, 2), "ne 2\nLine 3\nLi");
 }
 
 #include "DocumentContextReaderTest.moc"