Conversation
|
@gadenbuie I'm not allowed to push here, so here is a patch that fixes breakpoints with Ark. It moves the injection of the I've removed some old Windows-only code added in 89fe2ff to work around a bug that was fixed long ago (https://bugs.r-project.org/show_bug.cgi?id=16264). You'll also need posit-dev/ark#1041 to get From 2a0413e712ab8bea9a01b5f8a38cd37ec9dfbe18 Mon Sep 17 00:00:00 2001
From: Lionel Henry <lionel.hry@proton.me>
Date: Mon, 16 Feb 2026 15:25:36 +0100
Subject: [PATCH] Use pre-parse approach to inject `..stacktraceon..`
---
R/utils.R | 45 +++++++++++++++++++++++++--------------------
1 file changed, 25 insertions(+), 20 deletions(-)
diff --git a/R/utils.R b/R/utils.R
index 799bbf98f..6775e04fd 100644
--- a/R/utils.R
+++ b/R/utils.R
@@ -1397,30 +1397,35 @@ maybeAnnotateSourceForArk <- function(file, lines) {
# similarly, try to source() a file with UTF-8
sourceUTF8 <- function(file, envir = globalenv()) {
+ file <- normalizePath(file, mustWork = TRUE, winslash = "/")
+
lines <- readUTF8(file)
enc <- if (any(Encoding(lines) == 'UTF-8')) 'UTF-8' else 'unknown'
+
+ # Inject Ark annotations for breakpoints if available
lines <- maybeAnnotateSourceForArk(file, lines)
- src <- srcfilecopy(file, lines, isFile = TRUE) # source reference info
- # oddly, parse(file) does not work when file contains multibyte chars that
- # **can** be encoded natively on Windows (might be a bug in base R); we
- # rewrite the source code in a natively encoded temp file and parse it in this
- # case (the source reference is still pointed to the original file, though)
- if (isWindows() && enc == 'unknown') {
- file <- tempfile(); on.exit(unlink(file), add = TRUE)
- writeLines(lines, file)
- }
- exprs <- try(parse(file, keep.source = FALSE, srcfile = src, encoding = enc))
- if (inherits(exprs, "try-error")) {
- diagnoseCode(file)
- stop("Error sourcing ", file)
- }
- # Wrap the exprs in first `{`, then ..stacktraceon..(). It's only really the
- # ..stacktraceon..() that we care about, but the `{` is needed to make that
- # possible.
- exprs <- makeCall(`{`, exprs)
- # Need to wrap exprs in a list because we want it treated as a single argument
- exprs <- makeCall(..stacktraceon.., list(exprs))
+ # Wrap in `..stacktraceon..({...})` using string manipulation before parsing,
+ # with a `#line` directive to map source references back to the original file
+ lines <- c(
+ "..stacktraceon..({",
+ sprintf('#line 1 "%s"', file),
+ lines,
+ "})"
+ )
+
+ # Create a source file copy, i.e. an in-memory srcfile that contains all the
+ # code but refers to an original file
+ src <- srcfilecopy(file, lines, isFile = TRUE)
+
+ # Parse from our annotated lines
+ exprs <- tryCatch(
+ parse(text = lines, keep.source = FALSE, srcfile = src, encoding = enc),
+ error = function(cnd) {
+ diagnoseCode(file)
+ stop("Error sourcing ", file)
+ }
+ )
eval(exprs, envir)
} |
This comment was marked as spam.
This comment was marked as spam.
There was a problem hiding this comment.
Thanks @lionel-! (FYI you can ignore the CLA bot)
I tested locally but I don't have time atm to set up ark from the PR branch. I can tell though that the breakpoint is being activated as expected, which indicates this part of the fix is working.
R/utils.R
Outdated
| cli::cli_inform("Annotating source for Ark: {.val {uri}}") # FIXME | ||
| annotated <- ark_annotate_source(lines_str, uri) | ||
| if (!is.null(annotated)) { | ||
| lines <- strsplit(annotated, "\n", fixed = TRUE)[[1]] | ||
| } else { | ||
| cli::cli_warn("Ark annotation returned NULL") # FIXME | ||
| } |
There was a problem hiding this comment.
Remember to drop the FIXMEs before merging
R/utils.R
Outdated
| return(lines) | ||
| } | ||
|
|
||
| file <- normalizePath(file, mustWork = TRUE) # Just to be safe |
There was a problem hiding this comment.
Could drop this now that sourceUTF8() does this at the beginning? And if we end up keeping it We probably need the same winslash = "/"?
| # oddly, parse(file) does not work when file contains multibyte chars that | ||
| # **can** be encoded natively on Windows (might be a bug in base R); we | ||
| # rewrite the source code in a natively encoded temp file and parse it in this | ||
| # case (the source reference is still pointed to the original file, though) | ||
| if (isWindows() && enc == 'unknown') { | ||
| file <- tempfile(); on.exit(unlink(file), add = TRUE) | ||
| writeLines(lines, file) | ||
| } |
There was a problem hiding this comment.
The fix for this went into R 3.1.2. Let's update Depends to R (>= 3.1.2)?
cpsievert
left a comment
There was a problem hiding this comment.
LGTM once minor comments are addressed and NEWS.md is updated.
This PR introduces
maybeAnnotateSourceForArk(), which annotates source files for interactive debugging with Ark, primarily in Positron.Thanks to @lionel-, we also cleaned up some old Windows-only code that worked around a bug in R 3.1.2 (now fixed).