Turn rnvimserver into a language server by jalvesaq · Pull Request #449 · R-nvim/R.nvim

jalvesaq · 2025-11-06T19:56:05Z

No description provided.

Full of bugs, but the "resolve" works in some cases

Bug fixes

Add some TODOs Fix selene warning

Copilot

Pull Request Overview

Copilot reviewed 68 out of 69 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (4)

nvimcom/src/apps/utilities.c:1

Debug logging statement left in production code. This should either be removed or conditionally compiled based on a debug flag.
nvimcom/src/apps/logging.c:1
[nitpick] Silent failure when log file cannot be opened could make debugging difficult. Consider adding a fallback mechanism or one-time stderr warning.
nvimcom/src/apps/rnvimserver.c:1
Callback functions on_attach, on_init, on_exit, and on_error are only enabled when lsp_debug is true, but they still execute vim.notify() calls. These notifications should be gated or removed in production.
nvimcom/src/apps/rnvimserver.c:1
[nitpick] Magic number 380 used for truncating debug output should be defined as a named constant for clarity and maintainability.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-17T18:13:11Z

 local set_directories = function()
    -- config.rnvim_home should be the directory where the plugin files are.
-    config.rnvim_home = vim.fn.expand("<script>:h:h")
+    local rndir = debug.getinfo(1, "S").source


Pattern matching to extract plugin directory path could fail on Windows due to backslash path separators. Consider using vim.fn.fnamemodify() or normalizing path separators.

Suggested change

local rndir = debug.getinfo(1, "S").source

local rndir = debug.getinfo(1, "S").source

-- Normalize path separators to forward slashes for cross-platform compatibility

rndir = rndir:gsub("\\", "/")

Copilot · 2025-11-17T18:13:11Z

+    if not compl_region then
+        M.send_msg({ code = "E" .. req_id })
+        return
+    end


The compl_region variable is initialized to true but never modified elsewhere in the code. Document the purpose of this variable or remove it if unused.

PMassicotte · 2025-11-17T19:14:18Z

Cool. I will try that tonight.

should not be too long :)

Joke aside, very good job. I will play around a bit to see if I can spot any issues.

PMassicotte · 2025-11-17T20:02:00Z

One thing, could it be possible to complete data frame columns using the ~, like in ggplot2 facet_*() functions?

PMassicotte

Should we make a migration guide?

PMassicotte · 2025-11-17T20:22:13Z

+
+    -- check if the cursor is within comment or string
+    local snm = ""
+    local c = vim.treesitter.get_captures_at_pos(0, lnum, cnum - 1)


Suggested change

local c = vim.treesitter.get_captures_at_pos(0, lnum, cnum - 1)

local ok, c = pcall(vim.treesitter.get_captures_at_pos, 0, lnum, cnum - 1)

if not ok then c = {} end

Maybe replace all occurrences with similar check?

PMassicotte · 2025-11-17T20:31:11Z

Should we propose something like

return {
      {
          "R-nvim/R.nvim",
          version = "^1",  -- Use 1.x.x (new LSP)
          version = "^0",  -- Use old version with cmp-r
          config = function()
              require("r").setup({
                  ...
              })
          end,
      },
  }

jalvesaq · 2025-11-17T22:16:15Z

should not be too long :)

Too long! I hope we do not have to do any other transition like that in the next 20 years.

This PR induces breaking changes. Should we go to 1.0.0?

Not yet, I think. We are free to break compatibility with old code because we haven't released version 1.0.0 yet. I prefer to stay on version 0.99.x for a few months until we fix the bugs that will be reported. At least on Windows, I know there are bugs to be fixed. We can suggest a specific commit for anyone with problems with the "lsp" version. The last commit on the "main" branch was fef990378e4b5157f23314dca4136bc0079cc2c4. Alternatively, we can tag the last commit on the "main" branch as 0.99.1.

One thing, could it be possible to complete data frame columns using the ~, like in ggplot2 facet_*() functions?

Yes, editing the function need_R_args() (lua/r/lsp/init.lua). We may do this in the future, unless you have time to do it now. If we are going to implement this feature, I think it's better if 1.0.0 is released with it.

PMassicotte · 2025-11-17T22:25:09Z

Not yet, I think. We are free to break compatibility with old code because we haven't released version 1.0.0 yet. I prefer to stay on version 0.99.x for a few months until we fix the bugs that will be reported.

Totally fine with me.

I can take a look to the ~ implementation in the upcoming days. Good opportunity to get more used to the lsp code base.

Ready to get this big one merged? I have not found any issues lately.

PMassicotte · 2025-11-17T22:28:13Z

Maybe just add something in README to point to the latest main commit without built-in lsp. I expect a lot of new issues due to braking changes.

jalvesaq · 2025-11-17T22:47:39Z

Maybe just add something in README to point to the latest main commit without built-in lsp. I expect a lot of new issues due to braking changes.

I'll release 0.99.1 and the note to the README before merging the "lsp" branch. Then, wait for bug reports...

wurli · 2025-11-17T23:24:04Z

Congrats on getting this merged! A big step for R.nvim 🥳

PMassicotte · 2025-11-18T12:27:02Z

For some reason, I am not getting hover:

jalvesaq · 2025-11-18T12:37:28Z

Tree-sitter restriction:

(program ; [0, 0] - [1, 0]
  (identifier)) ; [0, 0] - [0, 6]

It only tries to get the summary if the object under the cursor is "variable". The purpose of this restriction was to avoid sending meaningless requests to R. Perhaps we should eliminate this restriction.

PMassicotte · 2025-11-18T12:40:37Z

It has a strange behavior, does not work on mtcars, neither on res. Does not work on lm unless it is on multiple lines.

jalvesaq · 2025-11-18T13:01:01Z

I created the branch "fix_lsp". We can fix minor bugs there for a few days, but merge it immediately if we fix a serious bug.

jalvesaq · 2025-11-18T13:06:23Z

On the "fix_lsp" branch, we have a new issue. The hover over data returns information on the function data(), which is incorrect:

res <- lm(mpg ~ wt + hp, data = mtcars)

jalvesaq · 2025-11-18T13:08:17Z

If we used tree-sitter information, we would know that data is @variable.parameter and that we don't support hover for function arguments.

jalvesaq · 2025-11-18T13:17:40Z

Perhaps you could make adjustments to the function hover() (lua/r/lsp/init.lua) to optimize it. The meanings of the parameters sent to rnvimserver are:

{ code = "N" .. req_id }: Null result.
{ code = "H", orig_id = req_id, word = word, fobj = first_obj }: word is a function; use the first object to check if we should show information for the function itself or for a method.
{ code = "H", orig_id = req_id, word = word }: try to find the appropriate information (might be a function without a first argument or anything else).

PMassicotte · 2025-11-18T13:23:15Z

Perhaps you could make adjustments to the function hover()

I think we could really benefit from using TS instead of string based search. What do you think? I do not mind to look at it, but will take few days, a lot of meetings/workshops to do this week on my side :)

jalvesaq · 2025-11-18T13:29:07Z

I think we could really benefit from using TS instead of string based search. What do you think? I do not mind to look at it, but will take few days, a lot of meetings/workshops to do this week on my side :)

Reading this now. I've said the same thing in the other thread. It's better to continue later, when we are both free.

jalvesaq added 30 commits October 18, 2025 15:15

LSP completion

b361f34

rnvimserver as LSP

085b56b

Full of bugs, but the "resolve" works in some cases

Escape documentation string for json

2ec9b4a

Use only letters for the 'cls' field

1b46cd7

Bug fixes

bug fixes

983013c

new get_word()

5dfd67f

disable valgrind

bbb6879

Improve rhelp keyword completion

a1e350d

Add some TODOs Fix selene warning

Completion of chunk options

4ab6d34

Correct way of setting environment variables

fbbea46

Handle shutdown method

9f6c2a3

Add missing files

4464c22

Add debugging functions: on_init and on_attach

b1d53bd

fix bug when completing function arguments

fe5d2b6

Disable on_init and on_attach

7c192aa

Move resolve function to resolve.c

d2b88c5

Move resolve() to resolve.c

0146049

Bug fixes

7ed477c

Bug fixes

3b129f7

Add r_ls config option

40bb14a

Implement resolve of chunk options

218abdb

Implement completion of @fig and @TBL

0e529b1

Complete arguments of .GlobalEnv functions

d4b4e99

Document some Lua functions

3b11a16

bug fixes

6d3e054

Fix \rh

ed19ed0

Fix highlighting of negative float number

9fbedd3

Minor optimizations

14151d7

Less verbose

9fbf34b

Improve formatting of doc (resolve event)

006ee9c

Update nvimcom version number for merging

6a3b046

jalvesaq marked this pull request as ready for review November 17, 2025 18:10

jalvesaq requested a review from Copilot November 17, 2025 18:11

Copilot AI reviewed Nov 17, 2025

View reviewed changes

Adjustments following Copilot suggestions

a7c71ba

PMassicotte reviewed Nov 17, 2025

View reviewed changes

Normalize RNVIM_HOME on Windows

788203f

jalvesaq added 3 commits November 17, 2025 19:57

Merge branch 'main' into lsp

82f7c90

Increase version number to 0.99.2

64d0350

Note on the README

51037b3

jalvesaq merged commit 9f21d6c into main Nov 17, 2025
9 checks passed

jalvesaq deleted the lsp branch November 17, 2025 23:20

wklimowicz mentioned this pull request Dec 20, 2025

Fix missing functions in help.R #478

Merged

	local c = vim.treesitter.get_captures_at_pos(0, lnum, cnum - 1)
	local ok, c = pcall(vim.treesitter.get_captures_at_pos, 0, lnum, cnum - 1)
	if not ok then c = {} end

Uh oh!

Conversation

jalvesaq commented Nov 6, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

PMassicotte commented Nov 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PMassicotte commented Nov 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PMassicotte left a comment

Choose a reason for hiding this comment

Uh oh!

PMassicotte Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

PMassicotte commented Nov 17, 2025

Uh oh!

jalvesaq commented Nov 17, 2025

Uh oh!

PMassicotte commented Nov 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PMassicotte commented Nov 17, 2025

Uh oh!

jalvesaq commented Nov 17, 2025

Uh oh!

Uh oh!

wurli commented Nov 17, 2025

Uh oh!

PMassicotte commented Nov 18, 2025

Uh oh!

jalvesaq commented Nov 18, 2025

Uh oh!

PMassicotte commented Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jalvesaq commented Nov 18, 2025

Uh oh!

jalvesaq commented Nov 18, 2025

Uh oh!

jalvesaq commented Nov 18, 2025

Uh oh!

jalvesaq commented Nov 18, 2025

Uh oh!

PMassicotte commented Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jalvesaq commented Nov 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

PMassicotte commented Nov 17, 2025 •

edited

Loading

PMassicotte commented Nov 17, 2025 •

edited

Loading

PMassicotte commented Nov 17, 2025 •

edited

Loading

PMassicotte commented Nov 18, 2025 •

edited

Loading

PMassicotte commented Nov 18, 2025 •

edited

Loading