Repository: helix-editor/helix
File: helix-core/src/diagnostic.rs (Rust)

Code before:
    pub provider: DiagnosticProvider,
    pub tags: Vec<DiagnosticTag>,
    pub source: Option<String>,
    pub data: Option<serde_json::Value>,
}

// TODO turn this into a feature flag when lsp becomes optional
#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
pub enum DiagnosticProvider {
    PublishDiagnosticProvider(LanguageServerId),
    PullDiagnosticProvider(LanguageServerId),
}

impl From<DiagnosticProvider> for LanguageServerId {
    fn from(value: DiagnosticProvider) -> Self {
        match value {
            DiagnosticProvider::PublishDiagnosticProvider(id) => id,
            DiagnosticProvider::PullDiagnosticProvider(id) => id,
        }
    }
}

// while I would prefer having this in helix-lsp that necessitates a bunch of
// conversions I would rather not add. I think its fine since this just a very
// trivial newtype wrapper and we would need something similar once we define
// completions in core
slotmap::new_key_type! {
    pub struct LanguageServerId;
}

impl fmt::Display for LanguageServerId {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        write!(f, "{:?}", self.0)
    }
}

impl Diagnostic {
    #[inline]
    pub fn severity(&self) -> Severity {
        self.severity.unwrap_or(Severity::Warning)
    }
}


Review comment (suggestion):
From the discussions I've read it sounds like this is meant to be done with the `identifier` value included on the server capabilities. R-a should add one in a future version https://github.com/rust-lang/rust-analyzer/pull/19266

So instead what I'm thinking for this enum is

```rust
pub enum DiagnosticProvider {
    Lsp {
        server_id: LanguageServerId,
        identifier: Option<String>,
    },
    // In the future, other non-LSP providers like spell checking go here...
}
```

where pull diagnostics would use that field from the capabilities and push diagnostics would always be `None`. (At least for now - the LSP spec could add an identifier to push diagnostics in the future.)

Code after:
    pub provider: DiagnosticProvider,
    pub tags: Vec<DiagnosticTag>,
    pub source: Option<String>,
    pub data: Option<serde_json::Value>,
}

/// The source of a diagnostic.
///
/// This type is cheap to clone: all data is either `Copy` or wrapped in an `Arc`.
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
pub enum DiagnosticProvider {
    Lsp {
        /// The ID of the language server which sent the diagnostic.
        server_id: LanguageServerId,
        /// An optional identifier under which diagnostics are managed by the client.
        ///
        /// `identifier` is a field from the LSP "Pull Diagnostics" feature meant to provide an
        /// optional "namespace" for diagnostics: a language server can respond to a diagnostics
        /// pull request with an identifier and these diagnostics should be treated as separate
        /// from push diagnostics. Rust-analyzer uses this feature for example to provide Cargo
        /// diagnostics with push and internal diagnostics with pull. The push diagnostics should
        /// not clear the pull diagnostics and vice-versa.
        identifier: Option<Arc<str>>,
    },
    // Future internal features can go here...
}

impl DiagnosticProvider {
    pub fn language_server_id(&self) -> Option<LanguageServerId> {
        match self {
            Self::Lsp { server_id, .. } => Some(*server_id),
            // _ => None,
        }
    }
}

// while I would prefer having this in helix-lsp that necessitates a bunch of
// conversions I would rather not add. I think its fine since this just a very
// trivial newtype wrapper and we would need something similar once we define
// completions in core
slotmap::new_key_type! {
    pub struct LanguageServerId;
}

impl fmt::Display for LanguageServerId {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        write!(f, "{:?}", self.0)
    }
}

impl Diagnostic {