-
Notifications
You must be signed in to change notification settings - Fork 253
Use Inferrable for target/ config option #793
Conversation
nrc
left a comment
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, a few comments inline
| return false; | ||
| } else { | ||
| return true; | ||
| } |
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 would be better as a 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.
Done. Let's, uh, not talk about previous, totally correct return false, though!
src/config.rs
Outdated
| return true; | ||
| } | ||
|
|
||
| return true; |
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 looks wrong - it's essentially if ... { true } else { true }
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.
And you don't need the final return
|
Thanks! |
Update RLS Includes rust-lang/rls#793 which fixes rust-lang/rls#803.
Followup to #788.
First of all, it fixes target/rls dir (with #788 it reused the same dir as Cargo, oops!).
It now uses the
Inferrablewrapper, so it's possible to specify the value, set it to fallback and then re-set it again.Additionally it properly uses the relative passed dir (which slipped through in #788, I definitely need to do something about the tests 😟 ) now.
r? @nrc