-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Adds sort for RecordLit comparison in SSR #3765
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
Conversation
crates/ra_ide/src/ssr.rs
Outdated
if pattern_fields.is_none() && code_fields.is_none() { | ||
return Some(match_); | ||
} | ||
|
||
if pattern_fields.is_none() || code_fields.is_none() { | ||
return None; | ||
} | ||
|
||
let mut pattern_fields = pattern_fields.unwrap().collect::<Vec<_>>(); | ||
let mut code_fields = code_fields.unwrap().collect::<Vec<_>>(); | ||
|
||
if pattern_fields.len() != code_fields.len() { | ||
return 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.
A single match might be better ? e.g.:
let (pattern_fields, code_fields) = match (pattern_fileds, code_fields) {
...
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.
I rewrote it little bit differently.
cc93943
to
8dd9777
Compare
) | ||
} | ||
|
||
fn check_opt_nodes( |
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.
I am going to use this functions for function call vs method call check.
|
||
fn check_opt_nodes( | ||
pattern: Option<impl AstNode>, | ||
code: Option<impl AstNode>, |
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 really need to pass these as Option
here? Seem like the all call sites of it could change to :
check_opt_nodes(pattern.path()?, code.path()?, placeholders, match_);
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.
I missed (None,None) arm, added
LGTM with a nit, but personally I don't like multiple long inline functions in rust but we could refactor it later on. |
8dd9777
to
47e8f3c
Compare
bors r+ Thanks @mikhail-m1 |
Build succeeded |
an item from #3186