Skip to content

Commit 16a39fc

Browse files
NerixyzNerixyz
authored
fix(spellcheck): use input highlighter's word segmentation to find word (#6822)
We used to use `QTextCursor`'s definition of a word when right-clicking. This doesn't necessarily align with ours (e.g. #6821 (comment)). Furthermore, we didn't consider the character after a word as part of it. Since Qt does some rounding, it might determine that you clicked on the character after a word if you clicked on the right side of it. With this PR, that's fixed. Here's a visualization of which characters we consider as part of a word (separated by even/odd): ``` ███ █████ ██████ ████ this is some text#I#write in here █████ █████ ██ ███ ``` Fixes #6819 Reported-by: pajlada <rasmus.karlsson@pajlada.com> Reviewed-by: pajlada <rasmus.karlsson@pajlada.com> Parent-pr: #6446
1 parent 5d19e66 commit 16a39fc

5 files changed

Lines changed: 143 additions & 5 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@
9595
- Dev: Unwrapped `LimitedQueueSnapshot` to `std::vector`. (#6606)
9696
- Dev: Simplified uses of `getMessageSnapshot`. (#6607)
9797
- Dev: Disabled `llvm-prefer-static-over-anonymous-namespace` in clang-tidy. (#6610)
98-
- Dev: Added experimental spell checker support. (#6446, #6703, #6722, #6730, #6731, #6779, #6780)
98+
- Dev: Added experimental spell checker support. (#6446, #6703, #6722, #6730, #6731, #6779, #6780, #6822)
9999
- Dev: Added Clazy linting in CI. (#6623)
100100
- Dev: Added custom clang-tidy module linting in CI. (#6626)
101101
- Dev: CMake option `USE_ALTERNATE_LINKER` now errors if the given linker can't be found. (#6692)

‎src/widgets/splits/InputHighlighter.cpp‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,41 @@ std::vector<QString> InputHighlighter::getSpellCheckedWords(const QString &text)
137137
return words;
138138
}
139139

140+
QStringView InputHighlighter::getWordAt(QStringView text, qsizetype pos)
141+
{
142+
auto tokenIt = this->tokenRegex.globalMatchView(text);
143+
QString token;
144+
qsizetype posInWord = 0;
145+
qsizetype tokenStart = 0;
146+
while (tokenIt.hasNext())
147+
{
148+
auto match = tokenIt.next();
149+
// using '<= end' to include the word left to the cursor if it's at the end
150+
if (match.capturedStart() <= pos && pos <= match.capturedEnd())
151+
{
152+
token = match.captured();
153+
tokenStart = match.capturedStart();
154+
posInWord = pos - tokenStart;
155+
break;
156+
}
157+
}
158+
if (token.isEmpty())
159+
{
160+
return {};
161+
}
162+
163+
QStringView word;
164+
this->visitWords(token, [&](const QString & /*curWord*/, qsizetype start,
165+
qsizetype count) {
166+
if (start <= posInWord && posInWord <= start + count)
167+
{
168+
assert(word.isEmpty());
169+
word = text.sliced(tokenStart + start, count);
170+
}
171+
});
172+
return word;
173+
}
174+
140175
void InputHighlighter::highlightBlock(const QString &text)
141176
{
142177
if (!this->spellChecker.isLoaded())

‎src/widgets/splits/InputHighlighter.hpp‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,10 @@ class InputHighlighter : public QSyntaxHighlighter
4343
/// checked by the spell checker.
4444
std::vector<QString> getSpellCheckedWords(const QString &text);
4545

46+
/// Get the word in \p text at \p pos. If there isn't any word, returns an
47+
/// empty view.
48+
QStringView getWordAt(QStringView text, qsizetype pos);
49+
4650
protected:
4751
void highlightBlock(const QString &text) override;
4852

‎src/widgets/splits/SplitInput.cpp‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -801,13 +801,27 @@ void SplitInput::installTextEditEvents()
801801
});
802802
menu->addAction(spellcheckAction);
803803

804-
auto cursor = this->ui_.textEdit->cursorForPosition(pos);
805-
cursor.select(QTextCursor::WordUnderCursor);
806-
auto word = cursor.selectedText();
804+
if (!this->inputHighlighter)
805+
{
806+
return;
807+
}
808+
809+
auto cursorAtPos = this->ui_.textEdit->cursorForPosition(pos);
810+
QString text = this->ui_.textEdit->toPlainText();
811+
QStringView word =
812+
this->inputHighlighter->getWordAt(text, cursorAtPos.position());
807813
if (!word.isEmpty())
808814
{
815+
auto cursor = this->ui_.textEdit->textCursor();
816+
// Select `word`. `word` is a view into `text`, so we can use
817+
// the offsets of `word` from the start of `text`.
818+
cursor.setPosition(
819+
static_cast<int>(word.begin() - text.begin()));
820+
cursor.setPosition(static_cast<int>(word.end() - text.begin()),
821+
QTextCursor::KeepAnchor);
822+
809823
auto suggestions =
810-
getApp()->getSpellChecker()->suggestions(word);
824+
getApp()->getSpellChecker()->suggestions(word.toString());
811825
for (const auto &sugg : suggestions)
812826
{
813827
auto qSugg = QString::fromStdString(sugg);

‎tests/src/InputHighlighter.cpp‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,3 +287,88 @@ TEST(InputHighlight, wordRegex)
287287
ASSERT_EQ(got, c.words) << "index=" << i;
288288
}
289289
}
290+
291+
TEST_F(InputHighlighterTest, getWordAt)
292+
{
293+
struct Case {
294+
std::pair<QStringView, qsizetype> input;
295+
QStringView word;
296+
};
297+
298+
std::vector<Case> cases{
299+
{.input = {u"", 0}, .word = {}},
300+
{.input = {u"", -1}, .word = {}},
301+
{.input = {u"", -2}, .word = {}},
302+
{.input = {u"", 1}, .word = {}},
303+
{.input = {u"", 2}, .word = {}},
304+
{.input = {u"word", -1}, .word = {}},
305+
{.input = {u"word", 0}, .word = u"word"},
306+
{.input = {u"word", 1}, .word = u"word"},
307+
{.input = {u"word", 2}, .word = u"word"},
308+
{.input = {u"word", 3}, .word = u"word"},
309+
{.input = {u"word", 4}, .word = u"word"},
310+
{.input = {u"word", 5}, .word = {}},
311+
{.input = {u"a word", -1}, .word = {}},
312+
{.input = {u"a word", 0}, .word = u"a"},
313+
{.input = {u"a word", 1}, .word = u"a"},
314+
{.input = {u"a word", 2}, .word = u"word"},
315+
{.input = {u"a word", 3}, .word = u"word"},
316+
{.input = {u"a word", 4}, .word = u"word"},
317+
{.input = {u"a word", 5}, .word = u"word"},
318+
{.input = {u"a word", 6}, .word = u"word"},
319+
{.input = {u"a word", 7}, .word = {}},
320+
{.input = {u"a word!", 6}, .word = u"word"},
321+
{.input = {u"a word!", 7}, .word = {}},
322+
{.input = {u"a word!", 8}, .word = {}},
323+
{.input = {u"three words are", 0}, .word = u"three"},
324+
{.input = {u"three words are", 4}, .word = u"three"},
325+
{.input = {u"three words are", 5}, .word = u"three"},
326+
{.input = {u"three words are", 6}, .word = u"words"},
327+
{.input = {u"three words are", 10}, .word = u"words"},
328+
{.input = {u"three words are", 11}, .word = u"words"},
329+
{.input = {u"three words are", 12}, .word = u"are"},
330+
{.input = {u"three words are", 14}, .word = u"are"},
331+
{.input = {u"three words are", 15}, .word = u"are"},
332+
{.input = {u"three words are", 16}, .word = {}},
333+
{.input = {u"a#text#is#here!", 0}, .word = u"a"},
334+
{.input = {u"a#text#is#here!", 1}, .word = u"a"},
335+
{.input = {u"a#text#is#here!", 2}, .word = u"text"},
336+
{.input = {u"a#text#is#here!", 5}, .word = u"text"},
337+
{.input = {u"a#text#is#here!", 6}, .word = u"text"},
338+
{.input = {u"a#text#is#here!", 7}, .word = u"is"},
339+
{.input = {u"a#text#is#here!", 8}, .word = u"is"},
340+
{.input = {u"a#text#is#here!", 9}, .word = u"is"},
341+
{.input = {u"a#text#is#here!", 10}, .word = u"here"},
342+
{.input = {u"a#text#is#here!", 13}, .word = u"here"},
343+
{.input = {u"a#text#is#here!", 14}, .word = u"here"},
344+
{.input = {u"a#text#is#here!", 15}, .word = {}},
345+
{.input = {u"here! there!", 3}, .word = u"here"},
346+
{.input = {u"here! there!", 4}, .word = u"here"},
347+
{.input = {u"here! there!", 5}, .word = {}},
348+
{.input = {u"here! there!", 6}, .word = u"there"},
349+
{.input = {u"/my-command wow", 0}, .word = {}},
350+
{.input = {u"/my-command wow", 1}, .word = {}},
351+
{.input = {u"/my-command wow", 2}, .word = {}},
352+
{.input = {u"/my-command wow", 10}, .word = {}},
353+
{.input = {u"/my-command wow", 11}, .word = {}},
354+
{.input = {u"/my-command wow", 12}, .word = u"wow"},
355+
{.input = {u"a 7TVGlobal b", 0}, .word = u"a"},
356+
{.input = {u"a 7TVGlobal b", 1}, .word = u"a"},
357+
{.input = {u"a 7TVGlobal b", 2}, .word = {}},
358+
{.input = {u"a 7TVGlobal b", 10}, .word = {}},
359+
{.input = {u"a 7TVGlobal b", 11}, .word = {}},
360+
{.input = {u"a 7TVGlobal b", 12}, .word = u"b"},
361+
{.input = {u"a 7TVGlobal b", 13}, .word = u"b"},
362+
{.input = {u"a 7TVGlobal b", 14}, .word = {}},
363+
};
364+
365+
SpellChecker nullSpellChecker;
366+
InputHighlighter highlighter(nullSpellChecker, nullptr);
367+
highlighter.setChannel(this->channel);
368+
for (size_t i = 0; i < cases.size(); i++)
369+
{
370+
const auto &c = cases[i];
371+
auto got = highlighter.getWordAt(c.input.first, c.input.second);
372+
ASSERT_EQ(got, c.word) << "index=" << i;
373+
}
374+
}

0 commit comments

Comments
 (0)