fix: Fix off by one errors in getContext{Before,After}() (#94)

This also specifies what exactly getContext*() functions do. Before this
commit linesCount was sometimes interpreted as exclusive of current
line, which was confusing as linesCount + 1 lines were being returned.
This commit is contained in:
Povilas Kanapickas 2025-03-05 20:32:53 +02:00 committed by GitHub
parent f9f2a86cea
commit 3dc0d910bf
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 57 additions and 20 deletions

View File

@ -84,11 +84,9 @@ QString DocumentContextReader::getLineText(int lineNumber, int cursorPosition) c
QString DocumentContextReader::getContextBefore(
int lineNumber, int cursorPosition, int linesCount) const
{
int effectiveStartLine;
int effectiveStartLine = qMax(0, lineNumber - linesCount + 1);
if (m_copyrightInfo.found) {
effectiveStartLine = qMax(m_copyrightInfo.endLine + 1, lineNumber - linesCount);
} else {
effectiveStartLine = qMax(0, lineNumber - linesCount);
effectiveStartLine = qMax(m_copyrightInfo.endLine + 1, effectiveStartLine);
}
return getContextBetween(effectiveStartLine, -1, lineNumber, cursorPosition);
@ -97,8 +95,8 @@ QString DocumentContextReader::getContextBefore(
QString DocumentContextReader::getContextAfter(
int lineNumber, int cursorPosition, int linesCount) const
{
int endLine = qMin(m_document->blockCount() - 1, lineNumber + linesCount);
return getContextBetween(lineNumber + 1, cursorPosition, endLine, -1);
int endLine = qMin(m_document->blockCount() - 1, lineNumber + linesCount - 1);
return getContextBetween(lineNumber, cursorPosition, endLine, -1);
}
QString DocumentContextReader::readWholeFileBefore(int lineNumber, int cursorPosition) const
@ -285,7 +283,7 @@ QString DocumentContextReader::getContextAfter(int lineNumber, int cursorPositio
int endLine = qMin(
m_document->blockCount() - 1,
lineNumber + Settings::codeCompletionSettings().readStringsAfterCursor());
return getContextBetween(lineNumber + 1, cursorPosition, endLine, -1);
return getContextBetween(lineNumber, cursorPosition, endLine, -1);
}
}

View File

@ -40,10 +40,31 @@ public:
QTextDocument *m_document, const QString &mimeType, const QString &filePath);
QString getLineText(int lineNumber, int cursorPosition = -1) const;
/**
* @brief Retrieves @c linesCount lines of context ending at @c lineNumber at
* @c cursorPosition in that line. The line at @c lineNumber is inclusive regardless of
* @c cursorPosition.
*/
QString getContextBefore(int lineNumber, int cursorPosition, int linesCount) const;
/**
* @brief Retrieves @c linesCount lines of context starting at @c lineNumber at
* @c cursorPosition in that line. The line at @c lineNumber is inclusive regardless of
* @c cursorPosition.
*/
QString getContextAfter(int lineNumber, int cursorPosition, int linesCount) const;
/**
* @brief Retrieves whole file ending at @c lineNumber at @c cursorPosition in that line.
*/
QString readWholeFileBefore(int lineNumber, int cursorPosition) const;
/**
* @brief Retrieves whole file starting at @c lineNumber at @c cursorPosition in that line.
*/
QString readWholeFileAfter(int lineNumber, int cursorPosition) const;
QString getLanguageAndFileInfo() const;
CopyrightInfo findCopyright();
QString getContextBetween(

View File

@ -62,38 +62,56 @@ TEST_F(DocumentContextReaderTest, testGetContextBefore)
{
auto reader = createTestReader("Line 1\nLine 2\nLine 3\nLine 4\nLine 5");
EXPECT_EQ(reader.getContextBefore(2, -1, 2), "Line 1\nLine 2\nLine 3");
EXPECT_EQ(reader.getContextBefore(4, -1, 3), "Line 2\nLine 3\nLine 4\nLine 5");
EXPECT_EQ(reader.getContextBefore(0, -1, 2), "Line 1");
EXPECT_EQ(reader.getContextBefore(1, -1, 2), "Line 1\nLine 2");
EXPECT_EQ(reader.getContextBefore(2, -1, 2), "Line 2\nLine 3");
EXPECT_EQ(reader.getContextBefore(3, -1, 2), "Line 3\nLine 4");
EXPECT_EQ(reader.getContextBefore(0, 1, 2), "L");
EXPECT_EQ(reader.getContextBefore(1, 1, 2), "Line 1\nL");
EXPECT_EQ(reader.getContextBefore(2, 1, 2), "Line 2\nL");
EXPECT_EQ(reader.getContextBefore(3, 1, 2), "Line 3\nL");
}
TEST_F(DocumentContextReaderTest, testGetContextAfter)
{
auto reader = createTestReader("Line 1\nLine 2\nLine 3\nLine 4\nLine 5");
EXPECT_EQ(reader.getContextAfter(0, -1, 2), "Line 2\nLine 3");
EXPECT_EQ(reader.getContextAfter(2, -1, 2), "Line 4\nLine 5");
EXPECT_EQ(reader.getContextAfter(0, -1, 2), "Line 1\nLine 2");
EXPECT_EQ(reader.getContextAfter(1, -1, 2), "Line 2\nLine 3");
EXPECT_EQ(reader.getContextAfter(2, -1, 2), "Line 3\nLine 4");
EXPECT_EQ(reader.getContextAfter(3, -1, 2), "Line 4\nLine 5");
EXPECT_EQ(reader.getContextAfter(0, 1, 2), "ine 1\nLine 2");
EXPECT_EQ(reader.getContextAfter(1, 1, 2), "ine 2\nLine 3");
EXPECT_EQ(reader.getContextAfter(2, 1, 2), "ine 3\nLine 4");
EXPECT_EQ(reader.getContextAfter(3, 1, 2), "ine 4\nLine 5");
}
TEST_F(DocumentContextReaderTest, testGetContextBeforeWithCopyright)
{
auto reader = createTestReader("/* Copyright (C) 2024 */\nLine 1\nLine 2\nLine 3\nLine 4");
// Test getting context before with copyright header
EXPECT_EQ(reader.getContextBefore(0, -1, 2), "");
EXPECT_EQ(reader.getContextBefore(1, -1, 2), "Line 1");
EXPECT_EQ(reader.getContextBefore(2, -1, 2), "Line 1\nLine 2");
// Test getting context before skipping copyright
EXPECT_EQ(reader.getContextBefore(3, 0, 2), "Line 1\nLine 2\n");
EXPECT_EQ(reader.getContextBefore(3, -1, 2), "Line 2\nLine 3");
EXPECT_EQ(reader.getContextBefore(0, 1, 2), "");
EXPECT_EQ(reader.getContextBefore(1, 1, 2), "L");
EXPECT_EQ(reader.getContextBefore(2, 1, 2), "Line 1\nL");
EXPECT_EQ(reader.getContextBefore(3, 1, 2), "Line 2\nL");
}
TEST_F(DocumentContextReaderTest, testGetContextAfterWithCopyright)
{
auto reader = createTestReader("/* Copyright (C) 2024 */\nLine 1\nLine 2\nLine 3\nLine 4");
// Test getting context after copyright header
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\nLine 3");
EXPECT_EQ(reader.getContextAfter(0, -1, 2), "/* Copyright (C) 2024 */\nLine 1");
EXPECT_EQ(reader.getContextAfter(1, -1, 2), "Line 1\nLine 2");
EXPECT_EQ(reader.getContextAfter(2, -1, 2), "Line 2\nLine 3");
EXPECT_EQ(reader.getContextAfter(3, -1, 2), "Line 3\nLine 4");
EXPECT_EQ(reader.getContextAfter(0, 1, 2), "* Copyright (C) 2024 */\nLine 1");
EXPECT_EQ(reader.getContextAfter(1, 1, 2), "ine 1\nLine 2");
EXPECT_EQ(reader.getContextAfter(2, 1, 2), "ine 2\nLine 3");
EXPECT_EQ(reader.getContextAfter(3, 1, 2), "ine 3\nLine 4");
}
TEST_F(DocumentContextReaderTest, testReadWholeFile)