Skip to content

Conversation

@etiennebacher
Copy link
Collaborator

@etiennebacher etiennebacher commented Nov 18, 2025

Part of #1397

Copilot AI review requested due to automatic review settings November 18, 2025 22:05
@etiennebacher etiennebacher marked this pull request as draft November 18, 2025 22:08
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements the <expr>$name$replace() method for replacing expression names using regular expressions or literal string matching.

  • Adds Rust bindings for the name().replace() method from Polars
  • Implements R wrapper function with regex and literal matching support
  • Includes comprehensive test coverage with regex patterns and literal strings

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
R/expr-name.R Adds expr_name_replace() function with pattern, value, and literal parameters
src/rust/src/expr/name.rs Implements Rust FFI binding for name_replace method
tests/testthat/test-expr-name.R Adds comprehensive tests covering regex and literal string replacement
man/expr_name_replace.Rd Generated documentation for the new method
R/000-wrappers.R Auto-generated R wrapper binding for Rust function
src/rust/api.h Auto-generated C API header declaration
src/init.c Auto-generated C initialization code for FFI
altdoc/mkdocs.yml Updates documentation index with new method
NEWS.md Documents new feature in changelog

#'
#'
#' @inheritParams rlang::args_dots_empty
#' @inheritParams cs__matches
Copy link

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

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

The @inheritParams cs__matches is inheriting malformed documentation. The cs__matches function uses Python/Sphinx-style documentation syntax that doesn't render properly in R documentation.

Consider either:

  1. Documenting the pattern parameter directly with proper roxygen2 syntax:
#' @param pattern A character or something can be coerced to a string [Expr]
#' of a valid regex pattern, compatible with the [regex crate](https://docs.rs/regex/latest/regex/).

Or 2. Inherit from a function that has proper documentation, such as expr_str_contains:

#' @inheritParams expr_str_contains

This would ensure the generated .Rd file has proper hyperlinks instead of malformed \verb{regex crate <...>}_ syntax.

Suggested change
#' @inheritParams cs__matches
#' @param pattern A character or something that can be coerced to a string [Expr] of a valid regex pattern, compatible with the [regex crate](https://docs.rs/regex/latest/regex/).

Copilot uses AI. Check for mistakes.
@etiennebacher etiennebacher marked this pull request as ready for review November 20, 2025 08:03
@eitsupi eitsupi merged commit 8b8448e into main Nov 20, 2025
27 checks passed
@eitsupi eitsupi deleted the name-replace branch November 20, 2025 14:06
@eitsupi eitsupi added this to the 1.7.0 milestone Nov 29, 2025
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.

3 participants