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

fix: various :RustAnalyzer target regressions #591

Merged
merged 4 commits into from
Nov 25, 2024

Conversation

MarkusPettersson98
Copy link
Contributor

This PR addresses three issues that would prevent a user from successfully running :RustAnalyzer target. It is pretty much a follow up to #589 and #548.

Each commit fixes one issue, and I will explain them in order:

1.bufnr argument missing when calling set_target_arch

:RustAnalyzer target would call set_target_arch by passing the target triple as the bufnr argument. Seems to be a regression introduced in #548 AFAICT.

2. Access non-existant key in rust-analyzer config / map

Inside set_target_arch, we would try to update a nested map (rust-analyzer) without first checking if each subkey was present. Specifically, client.settings['rust-analyzer'].cargo was nil, leading to the following statement failing catastrophically: client.settings['rust-analyzer'].cargo.target = target.

Curiously, it seems like rust-analyzer is present in client.config.settings and not client.settings, so I updated that. Works on my machine ¯\(ツ)/¯.

3. vim.notify was called at an inappropriate time

When set_target_arch is done updating the rust-analyzer config it will print a status update to the echo area using vim.notify. Apparently neovim didn't like this, and I got the following stacktrace after (successfully) calling :RustAnalyzer target aarch64-linux-android:

Error executing luv callback:
vim/_editor.lua:0: E5560: nvim_echo must not be called in a lua loop callback
stack traceback:
        [C]: in function 'nvim_echo'
        vim/_editor.lua: in function 'notify'
        .../rustaceanvim/lua/rustaceanvim/lsp/init.lua:321: in function 'on_exit'
        /usr/share/nvim/runtime/lua/vim/_system.lua:300: in function </usr/share/nvim/runtime/lua/vim/_system.lua:270>
        [C]: in function 'wait'
        /usr/share/nvim/runtime/lua/vim/_system.lua:98: in function 'wait'
        .../rustaceanvim/lua/rustaceanvim/cargo.lua:68: in function 'get_cargo_metadata'
        .../rustaceanvim/lua/rustaceanvim/cargo.lua:110: in function 'get_config_root_dir'
        .../rustaceanvim/lua/rustaceanvim/lsp/init.lua:159: in function 'start'
        .../rustaceanvim/lua/rustaceanvim/lsp/init.lua:121: in function <.../rustaceanvim/lua/rustaceanvim/lsp/init.lua:116>

Scheduling the status update with vim.schedule seems do have done the trick.

Final remarks

Sorry for not creating separate issues for all of the included fixes, but I patched the code as I went along and stumbled upon one error after the next. I love this package, and I hope that my work here will be useful to others. Cheers!

Copy link
Contributor

Review Checklist

Does this PR follow the Contribution Guidelines? Following is a partial checklist:

Proper conventional commit scoping:

  • For example, fix(lsp): some lsp-related bugfix

  • Pull request title has the appropriate conventional commit prefix.

If applicable:

  • Tested
    • Tests have been added.
    • Tested manually (Steps to reproduce in PR description).
  • Updated documentation.

mrcjkb
mrcjkb previously approved these changes Nov 25, 2024
Copy link
Owner

@mrcjkb mrcjkb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks 🙏

lua/rustaceanvim/lsp/init.lua Outdated Show resolved Hide resolved
@mrcjkb mrcjkb changed the title Fix: various :RustAnalyzer target errors fix: various :RustAnalyzer target regressions Nov 25, 2024
@mrcjkb mrcjkb enabled auto-merge (squash) November 25, 2024 20:03
@mrcjkb mrcjkb merged commit 4f62c30 into mrcjkb:master Nov 25, 2024
5 checks passed
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.

2 participants