From f9f2a86cea075913cf79595233b1f81a19f22306 Mon Sep 17 00:00:00 2001 From: Povilas Kanapickas Date: Wed, 5 Mar 2025 20:17:51 +0200 Subject: [PATCH] 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. --- context/DocumentContextReader.cpp | 81 ++++++++++++++++++++++++------ context/DocumentContextReader.hpp | 3 +- test/DocumentContextReaderTest.cpp | 25 ++++++--- 3 files changed, 87 insertions(+), 22 deletions(-) diff --git a/context/DocumentContextReader.cpp b/context/DocumentContextReader.cpp index b0ba207..2a569d4 100644 --- a/context/DocumentContextReader.cpp +++ b/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()) { - break; + return context; } - if (i == endLine) { - context += block.text().left(cursorPosition); + + 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 += block.text() + "\n"; + 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()) { + return context; + } + auto text = block.text(); + if (endCursorPosition < 0) { + context += text; + } else { + 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); } } diff --git a/context/DocumentContextReader.hpp b/context/DocumentContextReader.hpp index aa4a02a..fe021bc 100644 --- a/context/DocumentContextReader.hpp +++ b/context/DocumentContextReader.hpp @@ -46,7 +46,8 @@ public: 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; diff --git a/test/DocumentContextReaderTest.cpp b/test/DocumentContextReaderTest.cpp index d3b074a..2c3d654 100644 --- a/test/DocumentContextReaderTest.cpp +++ b/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"