Skip to content

Move footer function to cnd_footer() method#212

Merged
DavisVaughan merged 2 commits intomainfrom
fix/footer
Mar 5, 2026
Merged

Move footer function to cnd_footer() method#212
DavisVaughan merged 2 commits intomainfrom
fix/footer

Conversation

@DavisVaughan
Copy link
Copy Markdown
Member

@DavisVaughan DavisVaughan commented Feb 6, 2026

Closes #211

I've shifted us from using an on-the-fly created footer function to a cnd_footer() S3 method. This forces us to retain the minimal amount of things required to reproduce the footer message inside the cnd object, which seems to be the pkg name of interest and always.

We've got quite a few snapshot tests that test the emitted warning messages, so I feel pretty good about this.

library(dplyr)

tibble(
  x = 1:1000
) |> 
  cross_join(
    tibble(
      y = 1:1000
    )
  ) |> 
  mutate(
    v0 = "y"
  ) |> 
  mutate(
    v1 = case_match(v0, "blah" ~ "y")
  )

# BEFORE
lobstr::obj_size(dplyr:::the$last_warnings[[1]]$cnd)
#> 24.20 MB

# AFTER
lobstr::obj_size(dplyr:::the$last_warnings[[1]]$cnd)
#> 22.64 kB

Comment thread R/deprecate.R
)
}
}
trace <- trace_back(bottom = trace_env)
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I was a bit worried about the fact that trace is in the condition object, but it doesn't seem to be that big, so it's probably okay.

Comment thread R/warning.R
message = msg,
trace = trace,
footer = footer,
internal = list(...)
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We weren't using internal for anything

@DavisVaughan DavisVaughan requested a review from lionel- February 6, 2026 19:04
@DavisVaughan DavisVaughan merged commit 94c02eb into main Mar 5, 2026
14 checks passed
@DavisVaughan DavisVaughan deleted the fix/footer branch March 5, 2026 19:21
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.

Lifecycle footer function captures user's environment - which can result in massive memory explosion

2 participants