Skip to content
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

expose rust-analyzer lint errors from invalid Cargo.toml #61

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

falcucci
Copy link
Contributor

@falcucci falcucci commented Mar 16, 2024

currently the pop-up is shown as a generic error Failed to load workspaces in case you type a wrong crate name, for example, which makes it confusing to understand.

A more detailed notification would be appreciated.

Verified

This commit was signed with the committer’s verified signature.
falcucci Alexsander Falcucci
- remove the `stderr` handling from the `spawn` function
- add a function `send_error_notification` to send error notifications to all clients
- update the `stderr_task` function to accept an `instance` parameter
- add a call to `send_error_notification` within the `stderr_task` function
@pr2502
Copy link
Owner

pr2502 commented Mar 17, 2024

thanks! I'd like to try it in a few editors to see if we won't need to be a bit smarter about buffering the input more than by lines. So we don't get multiple notification bubbles for one error message spread across multiple lines, which could hide each other and make the message hard to read.

@pr2502
Copy link
Owner

pr2502 commented Mar 17, 2024

I'm trying to recreate the error messages you quoted in #60 (comment) but I can't 🤔

What editor are you using with ra-multiplex? There might be a capability it registers which helix does not for me. With the current rust-analyzer and logging notifications I got this in my logs:

ERROR instance{pid=19633}: stderr line=<timestamp> ERROR flycheck: Flycheck failed to run the following command: CommandHandle { program: "/usr/bin/cargo", arguments: ["check", "--workspace", "--message-format=json", "--manifest-path", "/home/max/code/ra-multiplex/Cargo.toml", "--all-targets"], current_dir: Some("/home/max/code/ra-multiplex") }
DEBUG instance{pid=19633}: server notification notif={"jsonrpc":"2.0","method":"$/progress","params":{"token":"rust-analyzer/flycheck/0","value":{"kind":"end"}}}
DEBUG instance{pid=19633}: server notification notif={"jsonrpc":"2.0","method":"window/showMessage","params":{"message":"cargo check failed to start: Cargo watcher failed, the command produced no valid metadata (exit code: ExitStatus(unix_wait_status(25856))):\n    Updating crates.io index\nerror: no matching package named `serde_jsonp` found\nlocation searched: registry `crates-io`\nrequired by package `ra-multiplex v0.2.2 (/home/max/code/ra-multiplex)`","type":2}}

I get a rather useless error in the stderr output of rust-analyzer but a very useful message with the window/showMessage method - in my case showing the stderr in the editor would not help at all.

What I might want to do, at least for my helix setup, is to allow ra-mulitplex to display notifications in system notifications via notify-send because helix apparently ignores window/showMessage notifications.

@falcucci
Copy link
Contributor Author

falcucci commented Mar 17, 2024

hm seems like rust-analyzer is using a different method to show up the message for you.

What editor are you using with ra-multiplex?

I am using neovim with coc.nvim and rust-analyzer at nightly.

notif: {"jsonrpc":"2.0","method":"$/progress","params":{"token":"rustAnalyzer/Fetching","value":{"kind":"end"}}}
notif: {"jsonrpc":"2.0","method":"experimental/serverStatus","params":{"health":"error","message":"Failed to load workspaces.","quiescent":false}}
notif: {"jsonrpc":"2.0","method":"$/progress","params":{"token":"rustAnalyzer/Fetching","value":{"cancellable":false,"kind":"begin","title":"Fetching"}}}
notif: {"jsonrpc":"2.0","method":"$/progress","params":{"token":"rustAnalyzer/Fetching","value":{"cancellable":false,"kind":"report","message":"metadata"}}}
ERROR instance{pid=63552}: stderr line=2024-03-17T14:02:21.999612Z ERROR rust_analyzer::main_loop: FetchWorkspaceError:
ERROR instance{pid=63552}: stderr line=rust-analyzer failed to load workspace: Failed to load the project at /Users/falcucci/neovide/Cargo.toml: Failed to read Cargo metadata from Cargo.toml file /Users/falcucci/neovide/Cargo.toml, Some(Version { major: 1, minor: 78, patch: 0, pre: Prerelease("nightly") }): Failed to run `cd "/Users/falcucci/neovide" && RUSTUP_TOOLCHAIN="/Users/falcucci/.rustup/toolchains/nightly-aarch64-apple-darwin" "/Users/falcucci/.cargo/bin/cargo" "metadata" "--format-version" "1" "--manifest-path" "/Users/falcucci/neovide/Cargo.toml" "--filter-platform" "aarch64-apple-darwin"`: `cargo metadata` exited with an error:     Updating crates.io index
ERROR instance{pid=63552}: stderr line=error: no matching package named `anyhow2` found
ERROR instance{pid=63552}: stderr line=location searched: registry `crates-io`
ERROR instance{pid=63552}: stderr line=required by package `neovide v0.12.2 (/Users/falcucci/neovide)`
ERROR instance{pid=63552}: stderr line=
ERROR instance{pid=63552}: stderr line=
notif: {"jsonrpc":"2.0","method":"$/progress","params":{"token":"rustAnalyzer/Fetching","value":{"kind":"end"}}}
notif: {"jsonrpc":"2.0","method":"experimental/serverStatus","params":{"health":"error","message":"Failed to load workspaces.","quiescent":true}}

as you can see it uses the "experimental/serverStatus" method here, yours has the ideal info message tho 😐

and in my case yes, the only way I could see more details in the message popup was notifying our stderr. It would be nice if I could filter the most useful one tho.

which rust-analyzer version are you using in helix?

@pr2502
Copy link
Owner

pr2502 commented Mar 17, 2024

I'm using rust-analyzer shipped by rustup

[language-server.rust-analyzer]
command = "/home/max/.cargo/bin/ra-multiplex"
args = ["client", "--server-path", "/usr/lib/rustup/bin/rust-analyzer"]

Which through rustup then launches /home/max/.rustup/toolchains/nightly-x86_64-unknown-linux/gnu/bin/rust-analyzer.

$ /usr/lib/rustup/bin/rust-analyzer --version
rust-analyzer 1.78.0-nightly (766bdce 2024-03-16)

@falcucci
Copy link
Contributor Author

@pr2502 that's super weird, I have the same rust-analyzer version. I also tried to reinstall just to make sure but it still uses "experimental/serverStatus" here.

It's important to note that zed editor have done something similar using the "experimental/serverStatus" aswell at https://github.com/zed-industries/zed/pull/8356/files which makes me think that it maybe varies from editor to editor? 👀

@falcucci
Copy link
Contributor Author

or we are missing something we didn't notice still

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants