R-nvim / R.nvim

Neovim plugin to edit R files
GNU General Public License v3.0
128 stars 15 forks source link

Restore debugging feature #165

Open jrwishart opened 4 days ago

jrwishart commented 4 days ago

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

jrwishart commented 4 days ago

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 commented 4 days ago

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 commented 4 days ago

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).

https://github.com/R-nvim/R.nvim/assets/4605715/dac96680-d4ea-4df7-883c-dc2f670a8cb1

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 commented 4 days ago

Might be related to https://github.com/R-nvim/R.nvim/issues/147. Perhaps nvimcom doesn't search functions that are sourced into the global env for the debug lines there?

jrwishart commented 4 days ago

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

PMassicotte commented 4 days ago

Peek 2024-06-29 08-22

PMassicotte commented 4 days ago

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

Peek 2024-06-29 09-15

PMassicotte commented 4 days ago

Maybe because I am using an external terminal?

jalvesaq commented 4 days ago

Some notes:

jalvesaq commented 4 days ago

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

jalvesaq commented 4 days ago

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 commented 4 days ago

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 commented 4 days ago

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.

https://github.com/R-nvim/R.nvim/assets/4605715/a663f124-7d62-4304-a953-4f8d1187db2d

jalvesaq commented 4 days ago

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 commented 3 days ago

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

jalvesaq commented 3 days ago

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 commented 3 days ago

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

jalvesaq commented 3 days ago

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 commented 3 days ago

@jrwishart, please, see https://github.com/R-nvim/R.nvim/discussions/166 as an alternative to this pull request...

jalvesaq commented 3 days ago

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 commented 2 days ago

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 commented 2 days ago

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.

https://github.com/R-nvim/R.nvim/assets/4605715/204a8ae3-8500-4664-9945-283a169ec5d9

jalvesaq commented 2 days ago

I think the problem is in line 90:

    if not s.debugging then

The buffer and window numbers are set only when the debugging session begins. I can look at this at the end of the week, but, please, feel free to fix it. It will be better for the R.nvim project if I'm not the only one to be able to maintain the debugging code...

jalvesaq commented 12 hours ago

Now, it should jump to other functions if you step into them.

jalvesaq commented 10 hours ago

If this pull request were merged, I believe the debug functionality of R.nvim would be as good (but also as limited) as that of Nvim-R. We only have to wait a few days for a working DAP for R before deciding either to merge or to close this pull request.

PMassicotte commented 10 hours ago

I agree. I will try this PR tomorrow when I am back to the office.

jrwishart commented 9 hours ago

Thanks for fixing. I was planning on having a close look after my business trip finishes at the end of the week but you were too fast again 🥇

I am happy to wait a little bit for the DAP situation to become clearer and avoid merging the PR here for now. The thread in https://github.com/dgkf/debugadapter looks promising. I'm willing to use R-devel if DAP works there. Can always pivot to using this PR if I need R release in the interim.