Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

add code for diagnostic. #3096

Merged
merged 2 commits into from
Jul 26, 2022
Merged

Conversation

erasin
Copy link
Contributor

@erasin erasin commented Jul 18, 2022

This PR provides a solution to resolve #2994. missing Code Actions for lsp

erasin added 2 commits July 18, 2022 23:22
This PR provides a solution to resolve helix-editor#2994. missing Code Actions for lsp
Comment on lines +81 to +87
let code = match diag.code.clone() {
Some(x) => match x {
NumberOrString::Number(x) => Some(lsp::NumberOrString::Number(x)),
NumberOrString::String(x) => Some(lsp::NumberOrString::String(x)),
},
None => None,
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to clone?

Copy link
Contributor Author

@erasin erasin Jul 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks , String not support Copy ., i remove clone

let code = match diag.code.as_ref() {
    Some(x) => match x {
        NumberOrString::Number(n) => Some(lsp::NumberOrString::Number(*n)),
        NumberOrString::String(s) => Some(lsp::NumberOrString::String(s.to_string())),
    },
    None => None,
};

I not sure how to write is better,

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, accidentally deleted a comment by @archseer. Please use clone, it's the same thing.

@erasin erasin force-pushed the lsp-miss-code-action branch from 8e4240e to a127242 Compare July 21, 2022 03:30
@helix-editor helix-editor deleted a comment from archseer Jul 23, 2022
Copy link
Contributor

@pickfire pickfire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

Comment on lines +26 to +30
#[derive(Debug, Eq, Hash, PartialEq, Clone, Deserialize, Serialize)]
pub enum NumberOrString {
Number(i32),
String(String),
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have to match lsp_types name using NumberOrString or would we be better off using DiagnosticCode?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it can express its meaning that is good.

@archseer archseer merged commit 42115d0 into helix-editor:master Jul 26, 2022
thomasskk pushed a commit to thomasskk/helix that referenced this pull request Sep 9, 2022
* add code for diagnostic.

This PR provides a solution to resolve helix-editor#2994. missing Code Actions for lsp

* remote unused import
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing Code Actions for TypeScript LSP
3 participants