Skip to content

Commit fedefae

Browse files
committed
Fix leaf-function extraction dropping part of the label
pv_print_hot_paths() stripped the trailing " (file:line)" from a stack entry with sub(" \\(.*\\)$", "", x). The greedy .* begins at the first " (" in the string, so any label containing a space before an open paren lost that part too: "if (x > 1) (file.R:3)" -> "if" The truncated name then fed is_user_function() and the pv_focus(p, "...") next-step suggestion, which could name a function that does not exist. Extracted strip_location() into utils.R, anchored on the file:line shape, which the location always has and a label never does.
1 parent 65307c8 commit fedefae

3 files changed

Lines changed: 22 additions & 1 deletion

File tree

R/hot-paths.R

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,9 @@ pv_print_hot_paths <- function(x, n = 10, include_source = TRUE) {
118118
# Extract the leaf function from the hottest path (last in the chain)
119119
parts <- strsplit(paths$stack[1], " -> ")[[1]]
120120
# Remove source info if present: "func (file:line)" -> "func"
121-
leaf_func <- sub(" \\(.*\\)$", "", parts[length(parts)])
121+
# Anchor on the file:line shape rather than ".*", so labels that themselves
122+
# contain " (" (e.g. "if (x > 1)") keep their parenthesised part.
123+
leaf_func <- strip_location(parts[length(parts)])
122124
suggestions <- "pv_flame(p)"
123125
if (is_user_function(leaf_func)) {
124126
suggestions <- c(sprintf("pv_focus(p, \"%s\")", leaf_func), suggestions)

R/utils.R

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,14 @@ cat_help_hint <- function() {
140140
cat("Run pv_help() to see all available functions.\n")
141141
}
142142

143+
# Strip a trailing " (file:line)" suffix from a stack entry, leaving the label.
144+
# The location never contains spaces or parens and always ends in ":<digits>",
145+
# so anchoring on that shape is safe even when the label itself contains " (",
146+
# as in "if (x > 1) (file.R:3)". A greedy " \\(.*\\)$" would eat both parts.
147+
strip_location <- function(entry) {
148+
sub(" \\([^ ()]+:[0-9]+\\)$", "", entry)
149+
}
150+
143151
# Check if a function name is a user function (not internal R machinery)
144152
# Internal functions start with: ( like (top-level), < like <GC>, [ like [.data.frame
145153
is_user_function <- function(func_name) {

tests/testthat/test-utils.R

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,3 +71,14 @@ test_that("get_source_lines returns NULL when start > end after clamping", {
7171
result <- debrief:::get_source_lines("R/main.R", 10, 2, contents)
7272
expect_null(result)
7373
})
74+
75+
test_that("strip_location keeps labels containing ' ('", {
76+
expect_equal(strip_location("paste (analysis.R:22)"), "paste")
77+
expect_equal(strip_location("if (x > 1) (file.R:3)"), "if (x > 1)")
78+
expect_equal(
79+
strip_location("data.frame(x = x, y = y) (analysis.R:11)"),
80+
"data.frame(x = x, y = y)"
81+
)
82+
expect_equal(strip_location("plain_label"), "plain_label")
83+
expect_equal(strip_location("f(a) (b)"), "f(a) (b)")
84+
})

0 commit comments

Comments
 (0)