Skip to content

Commit d5e140c

Browse files
committed
#3467 lsp: fix protocol bugs and marker issues
Signed-off-by: Patrizio Bekerle <[email protected]>
1 parent 16c4e1a commit d5e140c

5 files changed

Lines changed: 83 additions & 153 deletions

File tree

CHANGELOG.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,16 @@
1616
context menus; the old standalone **Code actions** menu item in the
1717
**Markdown LSP** submenu has been removed in favour of this inline approach
1818
(for [#3467](https://github.com/pbek/QOwnNotes/issues/3467))
19+
- Fixed a protocol bug where the Markdown LSP client did not respond to
20+
server-initiated requests (e.g. `client/registerCapability`), causing the
21+
LSP server to stall after the initial `didOpen`; subsequent `didChange`
22+
notifications were silently ignored by the server so diagnostics never
23+
refreshed after the first note load (for [#3467](https://github.com/pbek/QOwnNotes/issues/3467))
24+
- Fixed stale Markdown LSP wave underlines remaining visible after applying
25+
a quick-fix or manually editing the text to resolve a diagnostic; when fresh
26+
diagnostics arrive from the server, blocks that previously had diagnostics
27+
are now also rehighlighted so their old underlines are cleared
28+
(for [#3467](https://github.com/pbek/QOwnNotes/issues/3467))
1929

2030
## 26.4.23
2131

src/helpers/qownnotesmarkdownhighlighter.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -697,8 +697,6 @@ void QOwnNotesMarkdownHighlighter::highlightMarkdownLsp(const QString &text) {
697697
return;
698698
}
699699

700-
qDebug() << "LSP highlightMarkdownLsp: block" << blockNumber << "entries:" << it->size()
701-
<< "text:" << text.left(40);
702700
for (const LspBlockDiagnostic &entry : *it) {
703701
const int start = entry.startCharacter;
704702
const int end = (entry.endCharacter == INT_MAX) ? text.length() : entry.endCharacter;

src/services/markdownlspclient.cpp

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -472,7 +472,20 @@ void MarkdownLspClient::parseMessages() {
472472
}
473473

474474
const QJsonObject object = doc.object();
475-
if (object.contains(QStringLiteral("id"))) {
475+
const bool hasId = object.contains(QStringLiteral("id"));
476+
const bool hasMethod = object.contains(QStringLiteral("method"));
477+
478+
if (hasId && hasMethod) {
479+
// Server-to-client request (e.g. client/registerCapability,
480+
// window/workDoneProgress/create). Acknowledge with an empty
481+
// success response so the server doesn't stall waiting for it.
482+
const QJsonValue requestId = object.value(QStringLiteral("id"));
483+
QJsonObject response;
484+
response.insert(QStringLiteral("jsonrpc"), QStringLiteral("2.0"));
485+
response.insert(QStringLiteral("id"), requestId);
486+
response.insert(QStringLiteral("result"), QJsonValue());
487+
sendMessage(response);
488+
} else if (hasId) {
476489
handleResponse(object);
477490
} else {
478491
handleNotification(object);
@@ -631,7 +644,6 @@ void MarkdownLspClient::handleNotification(const QJsonObject &object) {
631644
diagnostics.push_back(diagnostic);
632645
}
633646

634-
qDebug() << "Markdown LSP diagnostics received for" << uri << "- count:" << diagnostics.size();
635647
emit diagnosticsReceived(uri, diagnostics);
636648
}
637649

@@ -692,14 +704,22 @@ QVector<MarkdownLspClient::TextEdit> MarkdownLspClient::parseWorkspaceEdits(
692704
return edits;
693705
}
694706

707+
const QString normalizedUri = QUrl::fromPercentEncoding(uri.toUtf8());
708+
695709
const QJsonValue changesValue = workspaceEdit.value(QStringLiteral("changes"));
696710
if (changesValue.isObject()) {
697711
const QJsonObject changesObject = changesValue.toObject();
698-
const QJsonValue uriEdits = changesObject.value(uri);
699-
if (uriEdits.isArray()) {
700-
edits = parseTextEdits(uriEdits);
701-
if (!edits.isEmpty()) {
702-
return edits;
712+
for (auto it = changesObject.constBegin(); it != changesObject.constEnd(); ++it) {
713+
const QString changeUri = it.key();
714+
if (QUrl::fromPercentEncoding(changeUri.toUtf8()) != normalizedUri) {
715+
continue;
716+
}
717+
718+
if (it.value().isArray()) {
719+
edits = parseTextEdits(it.value());
720+
if (!edits.isEmpty()) {
721+
return edits;
722+
}
703723
}
704724
}
705725
}
@@ -716,7 +736,7 @@ QVector<MarkdownLspClient::TextEdit> MarkdownLspClient::parseWorkspaceEdits(
716736
const QJsonObject textDocument =
717737
docChange.value(QStringLiteral("textDocument")).toObject();
718738
const QString changeUri = textDocument.value(QStringLiteral("uri")).toString();
719-
if (changeUri != uri) {
739+
if (QUrl::fromPercentEncoding(changeUri.toUtf8()) != normalizedUri) {
720740
continue;
721741
}
722742

src/widgets/qownnotesmarkdowntextedit.cpp

Lines changed: 44 additions & 137 deletions
Original file line numberDiff line numberDiff line change
@@ -2446,9 +2446,14 @@ void QOwnNotesMarkdownTextEdit::updateSettings() {
24462446

24472447
void QOwnNotesMarkdownTextEdit::onContextMenu(QPoint pos) {
24482448
auto *spellCheckMenu = spellCheckContextMenu(pos);
2449+
const QTextCursor cursorAtMouse = cursorForPosition(pos);
2450+
auto *lspMenu = markdownLspContextMenu(cursorAtMouse);
24492451

24502452
const QPoint globalPos = this->viewport()->mapToGlobal(pos);
24512453
QMenu *menu = this->createStandardContextMenu();
2454+
if (lspMenu) {
2455+
menu->insertMenu(menu->actions().constFirst(), lspMenu);
2456+
}
24522457
if (spellCheckMenu) {
24532458
// insert spell check at the top if available
24542459
menu->insertMenu(menu->actions().constFirst(), spellCheckMenu);
@@ -2690,8 +2695,6 @@ QMenu *QOwnNotesMarkdownTextEdit::spellCheckContextMenu(QPoint pos) {
26902695
addHarperMenuSection(menu, cursorAtMouse, cursor, hasEntries);
26912696
#endif
26922697

2693-
addMarkdownLspMenuSection(menu, cursorAtMouse, hasEntries);
2694-
26952698
if (!spellchecker || !spellchecker->isActive() || _isSpellCheckingDisabled) {
26962699
if (!hasEntries) {
26972700
delete menu;
@@ -2798,11 +2801,9 @@ QMenu *QOwnNotesMarkdownTextEdit::spellCheckContextMenu(QPoint pos) {
27982801
return menu;
27992802
}
28002803

2801-
void QOwnNotesMarkdownTextEdit::addMarkdownLspMenuSection(QMenu *menu,
2802-
const QTextCursor &cursorAtMouse,
2803-
bool &hasEntries) {
2804+
QMenu *QOwnNotesMarkdownTextEdit::markdownLspContextMenu(const QTextCursor &cursorAtMouse) {
28042805
if (!_markdownLspEnabled || !_markdownLspClient || _markdownLspDiagnostics.isEmpty()) {
2805-
return;
2806+
return nullptr;
28062807
}
28072808

28082809
// Find a diagnostic whose range covers the cursor position
@@ -2826,25 +2827,15 @@ void QOwnNotesMarkdownTextEdit::addMarkdownLspMenuSection(QMenu *menu,
28262827
}
28272828

28282829
if (!match) {
2829-
return;
2830-
}
2831-
2832-
// Add separator before LSP section if other entries already exist
2833-
if (hasEntries) {
2834-
menu->addSeparator();
2830+
return nullptr;
28352831
}
28362832

2837-
// Show the diagnostic message as a disabled header
2838-
const QString header = tr("Markdown LSP: %1").arg(match->message);
2839-
QAction *headerAction = menu->addAction(header);
2840-
headerAction->setEnabled(false);
2833+
auto *lspMenu = new QMenu(match->message, this);
28412834

28422835
// Request code actions for this diagnostic and wait briefly for the response
28432836
// using a local event loop (same pattern as QDialog::exec)
2844-
MarkdownLspClient::DiagnosticRange range = match->range;
2845-
_markdownLspLastCodeActions.clear();
28462837
_markdownLspCodeActionRequestId =
2847-
_markdownLspClient->requestCodeActions(_markdownLspUri, range, {*match});
2838+
_markdownLspClient->requestCodeActions(_markdownLspUri, match->range, {*match});
28482839

28492840
if (_markdownLspCodeActionRequestId >= 0) {
28502841
QVector<MarkdownLspClient::CodeAction> receivedActions;
@@ -2867,24 +2858,19 @@ void QOwnNotesMarkdownTextEdit::addMarkdownLspMenuSection(QMenu *menu,
28672858

28682859
if (receivedId == _markdownLspCodeActionRequestId) {
28692860
_markdownLspCodeActionRequestId = -1;
2870-
if (receivedActions.isEmpty()) {
2871-
QAction *noFixAction = menu->addAction(tr("No fixes available"));
2872-
noFixAction->setEnabled(false);
2873-
} else {
2874-
for (const auto &action : qAsConst(receivedActions)) {
2875-
menu->addAction(action.title, this, [this, action]() {
2876-
if (action.hasEdits()) {
2877-
applyMarkdownLspTextEdits(action.edits);
2878-
} else if (action.hasCommand()) {
2879-
_markdownLspClient->executeCommand(action.command);
2880-
}
2881-
});
2882-
}
2861+
for (const auto &action : qAsConst(receivedActions)) {
2862+
lspMenu->addAction(action.title, this, [this, action]() {
2863+
if (action.hasEdits()) {
2864+
applyMarkdownLspTextEdits(action.edits);
2865+
} else if (action.hasCommand()) {
2866+
_markdownLspClient->executeCommand(action.command);
2867+
}
2868+
});
28832869
}
28842870
}
28852871
}
28862872

2887-
hasEntries = true;
2873+
return lspMenu;
28882874
}
28892875

28902876
#ifdef LANGUAGETOOL_ENABLED
@@ -3515,10 +3501,8 @@ void QOwnNotesMarkdownTextEdit::closeMarkdownLspDocument() {
35153501
_markdownLspVersion = 0;
35163502
_markdownLspDiagnostics.clear();
35173503

3518-
auto *h = dynamic_cast<QOwnNotesMarkdownHighlighter *>(highlighter());
3519-
if (h) {
3504+
if (auto *h = dynamic_cast<QOwnNotesMarkdownHighlighter *>(highlighter())) {
35203505
h->clearMarkdownLspDiagnostics();
3521-
h->rehighlight();
35223506
}
35233507
}
35243508

@@ -3549,8 +3533,6 @@ void QOwnNotesMarkdownTextEdit::applyMarkdownLspSettings() {
35493533
&QOwnNotesMarkdownTextEdit::showMarkdownLspCompletions);
35503534
connect(_markdownLspClient, &MarkdownLspClient::formattingReceived, this,
35513535
&QOwnNotesMarkdownTextEdit::applyMarkdownLspFormatting);
3552-
connect(_markdownLspClient, &MarkdownLspClient::codeActionsReceived, this,
3553-
&QOwnNotesMarkdownTextEdit::showMarkdownLspCodeActions);
35543536
connect(_markdownLspClient, &MarkdownLspClient::diagnosticsReceived, this,
35553537
&QOwnNotesMarkdownTextEdit::showMarkdownLspDiagnostics);
35563538
connect(_markdownLspClient, &MarkdownLspClient::errorMessage, this,
@@ -3697,38 +3679,34 @@ void QOwnNotesMarkdownTextEdit::showMarkdownLspDiagnostics(
36973679
const QString normalizedUri = QUrl::fromPercentEncoding(uri.toUtf8());
36983680
const QString normalizedCurrentUri = QUrl::fromPercentEncoding(_markdownLspUri.toUtf8());
36993681
if (normalizedUri != normalizedCurrentUri) {
3700-
qDebug() << "Markdown LSP: ignoring diagnostics for" << uri
3701-
<< "(current:" << _markdownLspUri << ")";
37023682
return;
37033683
}
37043684

3705-
qDebug() << "Markdown LSP: applying" << diagnostics.size() << "diagnostics for" << uri;
3706-
for (const auto &d : diagnostics) {
3707-
qDebug() << " diagnostic: severity" << d.severity << "line" << d.range.startLine << "chars"
3708-
<< d.range.startCharacter << "-" << d.range.endCharacter << "msg:" << d.message;
3709-
}
3710-
_markdownLspDiagnostics = diagnostics;
3711-
37123685
auto *h = dynamic_cast<QOwnNotesMarkdownHighlighter *>(highlighter());
3713-
qDebug() << "Markdown LSP: highlighter cast:" << (h ? "OK" : "FAILED")
3714-
<< "document:" << (document() ? "OK" : "NULL");
37153686
if (!h || !document()) {
3687+
_markdownLspDiagnostics = diagnostics;
37163688
return;
37173689
}
37183690

3691+
// Collect blocks that had old diagnostics so we can clear their underlines
3692+
QSet<int> dirtyLines;
3693+
for (const MarkdownLspClient::Diagnostic &diag : _markdownLspDiagnostics) {
3694+
for (int line = diag.range.startLine; line <= diag.range.endLine; ++line) {
3695+
dirtyLines.insert(line);
3696+
}
3697+
}
3698+
3699+
_markdownLspDiagnostics = diagnostics;
37193700
h->setMarkdownLspDiagnostics(diagnostics);
37203701

3721-
// Rehighlight every block touched by a diagnostic so the underlines appear
3722-
// via QSyntaxHighlighter::setFormat() — identical to LanguageTool/Harper
3723-
QSet<int> dirtyLines;
3702+
// Also collect blocks that have new diagnostics
37243703
for (const MarkdownLspClient::Diagnostic &diag : diagnostics) {
37253704
for (int line = diag.range.startLine; line <= diag.range.endLine; ++line) {
37263705
dirtyLines.insert(line);
37273706
}
37283707
}
37293708

37303709
if (dirtyLines.isEmpty()) {
3731-
h->rehighlight();
37323710
return;
37333711
}
37343712

@@ -3785,6 +3763,14 @@ void QOwnNotesMarkdownTextEdit::applyMarkdownLspTextEdits(
37853763
return left.start > right.start;
37863764
});
37873765

3766+
// Clear the diagnostic cache before applying edits so that the rehighlight
3767+
// triggered by endEditBlock() will not re-apply the old LSP underlines.
3768+
_markdownLspDiagnostics.clear();
3769+
3770+
if (auto *h = dynamic_cast<QOwnNotesMarkdownHighlighter *>(highlighter())) {
3771+
h->clearMarkdownLspDiagnostics();
3772+
}
3773+
37883774
QTextCursor cursor(document());
37893775
cursor.beginEditBlock();
37903776
for (const ResolvedEdit &edit : resolved) {
@@ -3797,7 +3783,11 @@ void QOwnNotesMarkdownTextEdit::applyMarkdownLspTextEdits(
37973783
_markdownLspApplyingEdits = false;
37983784

37993785
if (_markdownLspEnabled && !_markdownLspApplyingEdits) {
3800-
scheduleMarkdownLspChange();
3786+
// Quick-fixes invalidate the current diagnostics immediately, so refresh
3787+
// the LSP state right away instead of waiting for the debounce timer.
3788+
_markdownLspPendingText = toPlainText();
3789+
_markdownLspChangeTimer->stop();
3790+
sendMarkdownLspChange();
38013791
}
38023792
}
38033793

@@ -3855,86 +3845,3 @@ void QOwnNotesMarkdownTextEdit::requestMarkdownLspFormatting(bool useSelection)
38553845
_markdownLspFormattingRequestId =
38563846
_markdownLspClient->requestDocumentFormatting(_markdownLspUri, tabSize, insertSpaces);
38573847
}
3858-
3859-
void QOwnNotesMarkdownTextEdit::requestMarkdownLspCodeActions(const QTextCursor &cursor) {
3860-
if (!_markdownLspEnabled || !_markdownLspClient || _markdownLspUri.isEmpty()) {
3861-
return;
3862-
}
3863-
3864-
if (_markdownLspDiagnostics.isEmpty()) {
3865-
return;
3866-
}
3867-
3868-
_markdownLspLastCodeActions.clear();
3869-
3870-
MarkdownLspClient::DiagnosticRange range;
3871-
if (cursor.hasSelection()) {
3872-
const int startPos = cursor.selectionStart();
3873-
const int endPos = cursor.selectionEnd();
3874-
const QTextBlock startBlock = document()->findBlock(startPos);
3875-
const QTextBlock endBlock = document()->findBlock(endPos);
3876-
if (!startBlock.isValid() || !endBlock.isValid()) {
3877-
return;
3878-
}
3879-
range.startLine = startBlock.blockNumber();
3880-
range.startCharacter = startPos - startBlock.position();
3881-
range.endLine = endBlock.blockNumber();
3882-
range.endCharacter = endPos - endBlock.position();
3883-
} else {
3884-
range.startLine = cursor.blockNumber();
3885-
range.startCharacter = cursor.positionInBlock();
3886-
range.endLine = range.startLine;
3887-
range.endCharacter = range.startCharacter;
3888-
}
3889-
3890-
_markdownLspCodeActionRequestId =
3891-
_markdownLspClient->requestCodeActions(_markdownLspUri, range, _markdownLspDiagnostics);
3892-
}
3893-
3894-
void QOwnNotesMarkdownTextEdit::showMarkdownLspCodeActions(
3895-
int requestId, const QVector<MarkdownLspClient::CodeAction> &actions) {
3896-
if (requestId != _markdownLspCodeActionRequestId || _markdownLspCodeActionRequestId == -1) {
3897-
return;
3898-
}
3899-
3900-
_markdownLspCodeActionRequestId = -1;
3901-
3902-
if (actions.isEmpty()) {
3903-
return;
3904-
}
3905-
3906-
_markdownLspLastCodeActions = actions;
3907-
3908-
QMenu menu;
3909-
for (int i = 0; i < actions.size(); ++i) {
3910-
const MarkdownLspClient::CodeAction &action = actions.at(i);
3911-
auto *menuAction = menu.addAction(action.title);
3912-
menuAction->setData(i);
3913-
menuAction->setWhatsThis(QStringLiteral("markdownLspCodeAction"));
3914-
}
3915-
3916-
if (menu.actions().isEmpty()) {
3917-
return;
3918-
}
3919-
3920-
QPoint globalPos = mapToGlobal(cursorRect().bottomRight());
3921-
globalPos.setY(globalPos.y() + viewportMargins().top());
3922-
globalPos.setX(globalPos.x() + viewportMargins().left());
3923-
3924-
QAction *selectedItem = menu.exec(globalPos);
3925-
if (!selectedItem) {
3926-
return;
3927-
}
3928-
3929-
const int index = selectedItem->data().toInt();
3930-
if (index < 0 || index >= _markdownLspLastCodeActions.size()) {
3931-
return;
3932-
}
3933-
3934-
const MarkdownLspClient::CodeAction &action = _markdownLspLastCodeActions.at(index);
3935-
if (action.hasEdits()) {
3936-
applyMarkdownLspTextEdits(action.edits);
3937-
} else if (action.hasCommand()) {
3938-
_markdownLspClient->executeCommand(action.command);
3939-
}
3940-
}

0 commit comments

Comments
 (0)