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

Restore debugging feature #165

Merged
merged 21 commits into from
Feb 17, 2025
Merged

Restore debugging feature #165

merged 21 commits into from
Feb 17, 2025

Conversation

jrwishart
Copy link
Contributor

Allow line highlighting in source buffer during browser and debug calls
of functions.

Allow line highlighting in source buffer during browser and debug calls
of functions.
@jrwishart
Copy link
Contributor Author

Apologies for the delay. Full time work was prioritised until this weekend. I removed some code paths that looked like it was unsupported tmux.

@PMassicotte
Copy link
Collaborator

Thank you for this PR!

I am testing it on my end, and while it appears to enter debug mode, I do not see any visual indication (e.g., line highlighting) of the debug process.

Peek 2024-06-29 06-45

@jrwishart
Copy link
Contributor Author

Thanks for the quick feedback.

Admittedly, I have never tried to debug a function sourced inline into the global environment. I have been doing it solely from a loaded package for package development. e.g. via devtools::load_all().

It works fine when functions are loaded (see video).

debug1080.mov

I can reproduce the same behaviour you have that the functions sourced inline in the console will not trigger the visual highlighting debug/browser.

@jrwishart
Copy link
Contributor Author

Might be related to #147. Perhaps nvimcom doesn't search functions that are sourced into the global env for the debug lines there?

@jrwishart
Copy link
Contributor Author

Just saw the CI checks failing. I'll see if I can fix those

doc/R.nvim.txt Outdated
Comment on lines 613 to 618
Another limitation of R-Nvim is that it might be unable to jump to the line in
the source code that is being evaluated if either you have sent the function
directly to the R Console or the function has been sent by the `knitr`
package, without calling the function `source()`. In either case, R-Nvim will
try to find the pattern `funcname <- function(` in the buffer being edited. If
the pattern is found, the cursor will jump to its position.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems like it was already a known implementation? (I didn't write the documentation in this PR, I am merely restoring it from the previous state).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just tried sourcing the function in a file using the source function and I get the same behaviour (no highlighting). So perhaps the documentation could be updated to reflect that loading functions into the environment via devtools will work but not using source?

Copy link
Collaborator

Choose a reason for hiding this comment

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

If I understand correctly, debugging will only work when sourcing a package with devtools.

@PMassicotte
Copy link
Collaborator

Peek 2024-06-29 08-22

@PMassicotte
Copy link
Collaborator

I am trying with a package, and still can not get it working. Any ideas?

Peek 2024-06-29 09-15

@PMassicotte
Copy link
Collaborator

PMassicotte commented Jun 29, 2024

Maybe because I am using an external terminal?

@jalvesaq
Copy link
Member

Some notes:

  • I have never used devtools. I only install it when someone reports a bug that requires it to be installed.
  • When using the debugging feature, I manually sourced the file with the function I wanted to debug. This way, R knew where the function was from.
  • The terminal emulator should not affect the process because nvimcom reads the R prompt (which indicates the debugging mode) directly from R's memory.

lua/r/debug.lua Outdated
Comment on lines 34 to 47
local rlines
local can_debug = type(config.external_term) == "boolean" and not config.external_term
if not can_debug then return end

s.func_offset = -1 -- Not found
local sbopt = vim.o.switchbuf
vim.o.switchbuf = "useopen,usetab"
local curtab = vim.fn.tabpagenr()
local isnormal = vim.fn.mode() == "n"
local curwin = vim.fn.winnr()
r_bufnr = vim.api.nvim_get_current_buf()
vim.cmd("sb " .. r_bufnr)
vim.fn.sleep(30) -- Time to fill the buffer lines
rlines = vim.fn.getline(1, "$")
Copy link
Contributor Author

@jrwishart jrwishart Jun 29, 2024

Choose a reason for hiding this comment

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

In this PR, the debug code only finds rlines (The R source lines I believe) if there is no external_term in the config.

There was vimscript code that defined rlines when debugging with tmux and I could look to convert it. That code is here.

R.nvim/R/start_r.vim

Lines 97 to 102 in 5b55186

elseif string(g:SendCmdToR) == "function('SendCmdToR_Term')"
let tout = system('tmux -L NvimR capture-pane -p -t ' . g:rplugin.tmuxsname)
let rlines = split(tout, "\n")
elseif string(g:SendCmdToR) == "function('SendCmdToR_TmuxSplit')"
let tout = system('tmux capture-pane -p -t ' . g:rplugin.rconsole_pane)
let rlines = split(tout, "\n")

I searched the commit history last week but couldn't determine what the proper way to do this in the lua plugin. As such I removed it. I mentioned it in the initial comment that I removed some code paths that looked like unsupported tmux.

Be happy to be advised on how to fix.

Copy link
Member

Choose a reason for hiding this comment

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

At this point, it's better to no longer look at Nvim-R code. What has to be done is relatively simple:

  1. When beginning the debugging, load the file where the code is or jump to it if it's already open (this is already done in the specific case that I fixed) and store the buffer number (not done yet).
  2. From the second jump onward, move the cursor without switching to its buffer (this will be different from Nvim-R).

@jalvesaq
Copy link
Member

I'm trying to fix the pull request...

…uns in a external terminal emulator

- Replace R-Nvim with `R.nvim` in the documentation.
- Rename `M.stop_r_debugging` as `M.stop`.
- Rename `M.find_debug_func` as `local find_func`.
- Rename `M.r_debug_jump` as `M.jump`.
- Fix bugs.
@jalvesaq
Copy link
Member

It's fixed on my side if R is running in an external terminal emulator and the function being debugged is in another script which was read with the source() command. Here's how to try it:

Uninstall nvimcom:

echo 'remove.packages("nvimcom")' | R

Then, follow the instructions from the documentation:

For example, if you have a file named my_function.R with the following contents:

add.one <- function(x) {
    y <- x + 1
    print(y)
    return(invisible(NULL))
}

You could debug the add.one() function from another script such as:

source("path/to/my_function.R")
debug(add.one)
add.one(7)

@jalvesaq
Copy link
Member

I removed "r" and "debug" from the names of Lua functions because the information that the function is related to debugging R functions is already present in the module name, "r.debug".

@jrwishart
Copy link
Contributor Author

Thanks for the quick update. I can confirm that it works for source now too 👍

I took the liberty to update the documentation to show the devtools approach. The only loose end is the last commit you made seems to have broken the cursor staying in the R console. Previously I could stay in the R console while debugging but now it is staying in the highlighted source line on every step requiring manually switching cursor to the R console each time.

debug_buf_switch.mov

@jalvesaq
Copy link
Member

The only loose end is the last commit you made seems to have broken the cursor staying in the R console.

I can look at this on the next weekend. I think the solution will be to implement steps 1 and 2 from my previous comment.

@PMassicotte
Copy link
Collaborator

No luck on my side.
Peek 2024-06-30 06-44

@jalvesaq
Copy link
Member

Try an R script with these lines:

source("debug.R")
debug(addone)
addone(1)

In its current buggy state, it only works if the function is in another script which still has to be open.

@PMassicotte
Copy link
Collaborator

Same thing, nothing happens.
Peek 2024-06-30 08-16

@jalvesaq
Copy link
Member

It only works if R is running in an external terminal emulator or a Tmux split pane because the code to open and switch buffers is yet to be fixed.

@jalvesaq
Copy link
Member

@jrwishart, please, see #166 as an alternative to this pull request...

- Update README (remove reference to no implementation of the debugging feature).
- Fix spelling of `debug_jump` option in `doc/R.nvim.txt`.
- Fix jumps form a buffer to another.
- Use `R_GetSrcFilename()` to simplify `nvimcom.c` code.
@jalvesaq
Copy link
Member

I found some time today and fixed the bugs I knew existed. Of course, instead of merging this pull request, it would be better to have a real DAP plugin.

@jrwishart
Copy link
Contributor Author

Thanks for the speedy reply/fix.

I agree that having dap would be preferable but it doesn't exist in a working state at the moment. (Happy to be proved otherwise)

@jrwishart
Copy link
Contributor Author

The last commit fixed the cursor position but seems to have introduced a new bug where it doesn't find the correct line if debug or browser is called from another function in a different file. E.g. in the video, bar calls hi, bar is in foo.R and hi is in qux.R and it gets the wrong line match for hi if it is a nested browser/debug call.

jump_480.mov

@PMassicotte
Copy link
Collaborator

It works on my side if I do srouce + debug(fun) in the terminal and then call the function. Should it works also if I place a browser() somewhere in a script and then use the send all lines to the terminal?

@jalvesaq
Copy link
Member

jalvesaq commented Jul 4, 2024

Thanks for catching this! It should work now.

@jalvesaq
Copy link
Member

jalvesaq commented Jul 4, 2024

When we source() a script, R knows where a function comes from and this information is available for us in an internal variable that can be accessed from C. When we send the function directly to R Console, R doesn't know where the function comes from. Then, we have to get the contents of R Console and look for the string "^debugging in: " or "Called from: ", extract the function name, and seek for it in the R script currently being edited.

So, the debugging will not work if you send a function directly to the R Console and switch to another buffer before starting to debug the function.

@jalvesaq
Copy link
Member

jalvesaq commented Jul 4, 2024

There is still one possible improvement. When debugging a function from an installed package or a function sent directly to the R Console from another R script, the source code isn't available, but the whole function is printed in the R Console, and we could copy it from there and paste it in a new temporary R script. However, before investing time in doing this, let's see if the DAP works...

@PMassicotte
Copy link
Collaborator

When we source() a script, R knows where a function comes from and this information is available for us in an internal variable that can be accessed from C. When we send the function directly to R Console, R doesn't know where the function comes from. Then, we have to get the contents of R Console and look for the string "^debugging in: " or "Called from: ", extract the function name, and seek for it in the R script currently being edited.

So, the debugging will not work if you send a function directly to the R Console and switch to another buffer before starting to debug the function.

Great this is working if I create a script like this:

addone <- function(x) {
  x + 1
}


browser()
addone(10)

and then source() it from the console.

@wurli
Copy link
Contributor

wurli commented Jul 7, 2024

Hi all, I've only been somewhat following this thread because I very rarely use R's debugger. But just want to throw this out there in case it's relevant - the guys at Posit are working on a DAP server/LSP server for R called Ark. It's not yet usable by other software than their new Positron IDE, but the README says it will be soon.

@jalvesaq
Copy link
Member

jalvesaq commented Jul 7, 2024

Thanks for the news, @wurli! This is another reason for not merging this pull request, but we can keep it open until either Ark or debugadapter is ready to be used in Neovim.

@akthe-at
Copy link
Contributor

@wurli @jalvesaq It looks like the Ark project is moving along and is starting to be usable outside of positron (Jupyter)...Very exciting times

@jalvesaq
Copy link
Member

Ark is still usable only by Positron.

https://codeberg.org/dgkf/debugadapter still says "Currently, nothing will happen" when "debugging".

Do we have other options? Should we keep waiting or merge this pull request?

@PMassicotte
Copy link
Collaborator

Have you been able to test it? Could be nice to have it rather than nothing. Maybe switch later when something else is ready.

@jalvesaq
Copy link
Member

I'm resolving the conflicts...

@jalvesaq
Copy link
Member

I fixed the conflicts in Github's online editor.

@PMassicotte
Copy link
Collaborator

Ok I will test it.

@PMassicotte
Copy link
Collaborator

Using this as testing code:

addone <- function(x) {
  browser()

  x + 1L

  z <- x / 42L
}


addone(10L)

for (x in 1L:3L) {
  print(x)
}

If I go in the console and type source("file.R"), it works.

However, I use <localleader>ae I get these errors:

Vim(lua):E5108: Error executing lua /media/LaCie16TB/work/dev/nvim/R.nvim/lua/r/debug.lua:56: attempt to call field 'system
' (a nil value)                                                                                                            
stack traceback:                                                                                                           
        /media/LaCie16TB/work/dev/nvim/R.nvim/lua/r/debug.lua:56: in function 'find_func'                                  
        /media/LaCie16TB/work/dev/nvim/R.nvim/lua/r/debug.lua:152: in function 'jump'                                      
        [string ":lua"]:1: in main chunk                                                                                   
        [C]: in function 'execute'                                                                                         
        /media/LaCie16TB/work/dev/nvim/R.nvim/lua/r/job.lua:48: in function 'exec_stdout_cmd'                              
        /media/LaCie16TB/work/dev/nvim/R.nvim/lua/r/job.lua:69: in function </media/LaCie16TB/work/dev/nvim/R.nvim/lua/r/jo
b.lua:58>                                                                                                                  
stack traceback:                                                                                                           
        [C]: in function 'execute'                                                                                         
        /media/LaCie16TB/work/dev/nvim/R.nvim/lua/r/job.lua:48: in function 'exec_stdout_cmd'                              
        /media/LaCie16TB/work/dev/nvim/R.nvim/lua/r/job.lua:69: in function </media/LaCie16TB/work/dev/nvim/R.nvim/lua/r/jo
b.lua:58>                                                                                                                  
Vim(lua):E5108: Error executing lua /media/LaCie16TB/work/dev/nvim/R.nvim/lua/r/debug.lua:56: attempt to call field 'system
' (a nil value)                                                                                                            
stack traceback:                                                                                                           
        /media/LaCie16TB/work/dev/nvim/R.nvim/lua/r/debug.lua:56: in function 'find_func'                                  
        /media/LaCie16TB/work/dev/nvim/R.nvim/lua/r/debug.lua:152: in function 'jump'                                      
        [string ":lua"]:1: in main chunk                                                                                   
        [C]: in function 'execute'                                                                                         
        /media/LaCie16TB/work/dev/nvim/R.nvim/lua/r/job.lua:48: in function 'exec_stdout_cmd'                              
        /media/LaCie16TB/work/dev/nvim/R.nvim/lua/r/job.lua:69: in function </media/LaCie16TB/work/dev/nvim/R.nvim/lua/r/jo
b.lua:58>                                                                                                                  
stack traceback:                                                                                                           
        [C]: in function 'execute'                                                                                         
        /media/LaCie16TB/work/dev/nvim/R.nvim/lua/r/job.lua:48: in function 'exec_stdout_cmd'                              
        /media/LaCie16TB/work/dev/nvim/R.nvim/lua/r/job.lua:69: in function </media/LaCie16TB/work/dev/nvim/R.nvim/lua/r/jo
b.lua:58> function: builtin#18 Vim(lua):E5108: Error executing lua /media/LaCie16TB/work/dev/nvim/R.nvim/lua/r/debug.lua:56
: attempt to call field 'system' (a nil value)                                                                             
stack traceback:                                                                                                           
        /media/LaCie16TB/work/dev/nvim/R.nvim/lua/r/debug.lua:56: in function 'find_func'                                  
        /media/LaCie16TB/work/dev/nvim/R.nvim/lua/r/debug.lua:152: in function 'jump'                                      
        [string ":lua"]:1: in main chunk                                                                                   
        [C]: in function 'execute'                                                                                         
        /media/LaCie16TB/work/dev/nvim/R.nvim/lua/r/job.lua:48: in function 'exec_stdout_cmd'                              
        /media/LaCie16TB/work/dev/nvim/R.nvim/lua/r/job.lua:69: in function </media/LaCie16TB/work/dev/nvim/R.nvim/lua/r/jo
b.lua:58>     

@jalvesaq
Copy link
Member

Fixed: utils.system() replaced with vim.system()

lua/r/debug.lua Outdated
"-t",
require("r.external_term").get_tmuxsname(),
}
local resp = require("r.utils").system(run_cmd, { text = true }):wait()
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is the faulty line when trying to source a file that includes a browser() call.

doc/R.nvim.txt Outdated Show resolved Hide resolved
doc/R.nvim.txt Outdated Show resolved Hide resolved
@PMassicotte
Copy link
Collaborator

Thanks, fixed small error on keymaping and added a sentence.

jalvesaq and others added 3 commits February 17, 2025 10:59
@PMassicotte PMassicotte merged commit 2f4b931 into R-nvim:main Feb 17, 2025
1 check passed
@PMassicotte
Copy link
Collaborator

Thank you and thanks to @jrwishart for this work.

jalvesaq added a commit that referenced this pull request Feb 17, 2025
* Restore debugging feature
* Update documentation to mention devtools approach

Co-authored-by: Jakson Alves de Aquino <[email protected]>
Co-authored-by: Philippe Massicotte <[email protected]>
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.

5 participants