Repository: helix-editor/helix
File: helix-term/src/handlers/diagnostics.rs (Rust)

Code before:
    }
}

fn handle_document_diagnostic_report_kind(
    editor: &mut Editor,
    document_id: DocumentId,
    report: Option<HashMap<lsp::Url, lsp::DocumentDiagnosticReportKind>>,
    provider: DiagnosticProvider,
) {
    for (url, report) in report.into_iter().flatten() {
        match report {
            lsp::DocumentDiagnosticReportKind::Full(report) => {
                let Ok(uri) = Uri::try_from(url) else {
                    return;
                };

                editor.handle_lsp_diagnostics(provider, uri, None, report.items, report.result_id);
            }
            lsp::DocumentDiagnosticReportKind::Unchanged(report) => {
                let Some(doc) = editor.document_mut(document_id) else {
                    return;
                };
                doc.previous_diagnostic_id = Some(report.result_id);
            }
        }
    }
}


Review comment (refactor):
I think we can eliminate the extra helper function here and make this a bit shorter

```rust
fn handle_pull_diagnostics_response(
    editor: &mut Editor,
    response: lsp::DocumentDiagnosticReport,
    provider: DiagnosticProvider,
    uri: Uri,
    document_id: DocumentId,
) {
    let (result_id, related_documents) = match response {
        lsp::DocumentDiagnosticReport::Full(report) => {
            editor.handle_lsp_diagnostics(
                provider,
                uri,
                None,
                report.full_document_diagnostic_report.items,
            );

            (
                report.full_document_diagnostic_report.result_id,
                report.related_documents,
            )
        }
        lsp::DocumentDiagnosticReport::Unchanged(report) => (
            Some(report.unchanged_document_diagnostic_report.result_id),
            report.related_documents,
        ),
    };

    if let Some(doc) = editor.document_mut(document_id) {
        doc.previous_diagnostic_id = result_id;
    };

    for (url, report) in related_documents.into_iter().flatten() {
        let result_id = match report {
            lsp::DocumentDiagnosticReportKind::Full(report) => {
                let Ok(uri) = Uri::try_from(url) else {
                    continue;
                };

                editor.handle_lsp_diagnostics(provider, uri, None, report.items);
                report.result_id
            }
            lsp::DocumentDiagnosticReportKind::Unchanged(report) => Some(report.result_id),
        };

        if let Some(doc) = editor.document_mut(document_id) {
            doc.previous_diagnostic_id = result_id;
        }
    }
}
```

I think we should also try to apply as much of the report (continue rather than return).

I also changed the signature of `Editor::handle_lsp_diagnostics` here so that the calling code here sets the `result_id` rather than passing it. I think that makes the option handling a little cleaner and we don't need to worry about the meaning of this field for push diagnostics.

Instead of passing the result_id though I believe we will need to pass the `identifier` (or at least the `DiagnosticProvider`).

Code after:

            Some(async move {
                let result = future.await;

                (result, provider, uri)
            })
        })
        .collect();

    if futures.is_empty() {
        return;
    }

    tokio::spawn(async move {
        let mut retry_language_servers = HashSet::new();
        loop {
            match cancelable_future(futures.next(), &cancel).await {
                Some(Some((Ok(result), provider, uri))) => {
                    job::dispatch(move |editor, _| {
                        handle_pull_diagnostics_response(editor, result, provider, uri, doc_id);
                    })
                    .await;
                }
                Some(Some((Err(err), DiagnosticProvider::Lsp { server_id, .. }, _))) => {
                    let parsed_cancellation_data = if let helix_lsp::Error::Rpc(error) = err {
                        error.data.and_then(|data| {
                            serde_json::from_value::<lsp::DiagnosticServerCancellationData>(data)
                                .ok()
                        })
                    } else {
                        log::error!("Pull diagnostic request failed: {err}");
                        continue;
                    };
                    if parsed_cancellation_data.is_some_and(|data| data.retrigger_request) {
                        retry_language_servers.insert(server_id);
                    }
                }
                Some(None) => break,
                // The request was cancelled.
                None => return,
            }
        }

        if !retry_language_servers.is_empty() {
            tokio::time::sleep(Duration::from_millis(500)).await;

            job::dispatch(move |editor, _| {
                request_document_diagnostics_for_language_severs(
                    editor,
                    doc_id,
                    retry_language_servers,