-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add code for diagnostic. #3096
add code for diagnostic. #3096
Conversation
This PR provides a solution to resolve helix-editor#2994. missing Code Actions for lsp
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, | ||
}; |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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,
There was a problem hiding this comment.
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.
8e4240e
to
a127242
Compare
There was a problem hiding this 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.
#[derive(Debug, Eq, Hash, PartialEq, Clone, Deserialize, Serialize)] | ||
pub enum NumberOrString { | ||
Number(i32), | ||
String(String), | ||
} |
There was a problem hiding this comment.
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
?
There was a problem hiding this comment.
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.
* add code for diagnostic. This PR provides a solution to resolve helix-editor#2994. missing Code Actions for lsp * remote unused import
This PR provides a solution to resolve #2994. missing Code Actions for lsp