-
Notifications
You must be signed in to change notification settings - Fork 1.8k
fix: Avoid .unwrap() when running the discover command #20126
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
fix: Avoid .unwrap() when running the discover command #20126
Conversation
b034b22
to
f20eec0
Compare
I feel like crashing is acceptable here? If we can't run the command, we can't discover the project, and r-a doesn't have a lot it can do. The choice of the temp dir also feels arbitrary and may cause weird misbehaviors. |
We don't crash when e.g. |
Logging a specific error and r-a continuing in detached file mode does feel more friendly than just "rust-analyzer crashed five times, it will not restart" which is the current behaviour. I noticed this when I'd configured the discover command with a local build, forgot about it, eventually cleaned up my build directory, and got very confused when my r-a setup broke. |
Regarding the fallback directory, I considered |
My weak opinion here is that we should probably crash five times if the provided argument doesn't exist/buck2 did some some garbage collection on Don't get me wrong: I don't think the current state is good, but we could probably make this suck less with an |
Note that logging an error here is basically the same as not reporting anything to the user, unless they realize something is wrong and dig through the logs. A panic is at least more noticeable, the other alternative would be to color the status bar as warning / error when this occurs (then the log is more meaningful) |
Another option would be to trigger Using |
Previously, the following configuration in settings.json: "rust-analyzer.workspace.discoverConfig": { "command": [ "oops", "develop-json", "{arg}" ], "progressLabel": "rust-analyzer", "filesToWatch": [ "BUCK", "TARGETS" ] }, Would previously cause a crash in rust-analyzer: thread 'LspServer' panicked at crates/rust-analyzer/src/main_loop.rs:776:84: called `Result::unwrap()` on an `Err` value: Os { code: 2, kind: NotFound, message: "No such file or directory" } Instead, use more specific panic messages.
f20eec0
to
a50b7b6
Compare
OK, I've updated to still panic, but with more specific messages. |
Previously, the following configuration in settings.json:
Would previously cause a crash in rust-analyzer:
Instead, use more specific panic messages.