-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Add more heuristics for hiding obvious param hints #3901
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
This will now hide "value", "pat", "rhs" and "other" These words were selected from the std because they are used in common functions with only a single param and are obvious by their use. I think it would be good to also hide "bytes" if the type is `[u8; n]` but I'm not sure how to get the param type signature It will also hide the hint if the passed param starts or end with the param_name
|
I added a way to ignore |
crates/ra_ide/src/inlay_hints.rs
Outdated
| ) -> bool { | ||
| let argument_string = argument.syntax().to_string(); | ||
| if param_name.is_empty() || argument_string.ends_with(param_name) { | ||
| let argument_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.
Indeed, this is better expressed as if let ast::RefExpr(argument) = argument.
Probably better expressed as a helper function:
fn remove_reference(exr: ast::Expr) -> ast::Expr
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.
So, I tried to make a function that strips the Ref part, but I ended up with a token which I didn't know how to convert back to an ast::Expr. Instead I made a fn is_argument_similar_to_param(argument, param_name) -> bool. It makes the intent pretty explicit, but it's less reusable.
crates/ra_ide/src/inlay_hints.rs
Outdated
| "predicate" | "value" | "pat" | "rhs" | "other" => true, | ||
| _ => false, | ||
| }; | ||
| if parameters_len == 1 && (param_name.len() == 1 || is_obvious_param_name) { |
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.
Let's extract the (param_name.len() == 1 || is_obvious_param_name) into a
fn is_obvious_param(param: &str) -> bool
function?
| @@ -1,4 +1,4 @@ | |||
| //! FIXME: write short doc here | |||
| //! This module defines multiple types of inlay hints and their visibility | |||
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.
😍
crates/ra_ide/src/inlay_hints.rs
Outdated
|
|
||
| fn is_argument_similar_to_param(argument: &ast::Expr, param_name: &str) -> bool { | ||
| let argument_string = if let ast::Expr::RefExpr(ref_expr) = argument { | ||
| ref_expr.syntax().last_token().expect("RefExpr should have a last_token").to_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.
This expect can in theory fire. I suggest doing this:
fn remove_ref(expr: ast::Expr) -> ast::Expr {
if let ast::Expr::RefExpr(ref_expr) = &expr {
if let Some(inner) = ref_expr.expr() {
return expr;
}
}
expr
}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 didn't realize there was a .expr() on the RefExpr. I'll fix that!
|
bors r+ Thanks! |
3901: Add more heuristics for hiding obvious param hints r=matklad a=IceSentry This will now hide `value`, `pat`, `rhs` and `other`. These words were selected from the std because they are used in commonly used functions with only a single param and are obvious by their use. It will also hide the hint if the passed param **starts** or end with the param_name. Maybe we could also split on '_' and check if one of the string is the param_name. I think it would be good to also hide `bytes` if the type is `[u8; n]` but I'm not sure how to get the param type signature. Closes #3900 Co-authored-by: IceSentry <c.giguere42@gmail.com>
Build failed |
|
I think the test failure is spurious. Can someone retry this? |
|
bors retry |
Build succeeded |
This will now hide
value,pat,rhsandother. These words were selected from the std because they are used in commonly used functions with only a single param and are obvious by their use.It will also hide the hint if the passed param starts or end with the param_name. Maybe we could also split on '_' and check if one of the string is the param_name.
I think it would be good to also hide
bytesif the type is[u8; n]but I'm not sure how to get the param type signature.Closes #3900