Conversation
- countDotOrUnderscoreSeparated() used dplyr::case_when() for a single call site; replaced with a plain base R if/else chain. Same behavior as before for every previously-handled case, plus it now fixes a real gap: any two-way tie among the dot/underscore/whitespace counts (e.g. "a.b_c.d_e f", where dots tie underscores at 2 and both beat the 1 whitespace) fell through all of case_when's branches and silently returned NA. It now resolves to "undecided", same as the pre-existing three-way-tie case. - dplyr is now unused; removed it from Imports/NAMESPACE and Development/config.R (which generates DESCRIPTION). - clipr is only ever used behind requireNamespace()/try() guards (an optional, best-effort feature), which is exactly what DESCRIPTION's Suggests field is for. Moved it from Imports to Suggests. Because a NAMESPACE importFrom() forces a hard dependency (R CMD check ERRORs if a package with an importFrom() sits in Suggests), also dropped the three @importFrom clipr write_clip roxygen tags; the code already calls clipr::write_clip() fully qualified, which is exactly the supported pattern for optional Suggests-only dependencies. - Fixed an unrelated pre-existing R CMD check NOTE surfaced during this work: ParseFullFilePath() calls hasArg(), which lives in the methods package (not base), but nothing declared that dependency. Added @importFrom methods hasArg and moved `methods` into Imports. Version bumped 1.2.1 -> 1.2.2 in Development/config.R and DESCRIPTION. Validation: R CMD check and the existing testthat suite (7/7 passing) were run on this branch. Remaining R CMD check findings are pre-existing and out of scope for this PR: the parFlags() example errors (fixed in a follow-up PR forwarding it to parFlags2()), parsepvalue()'s undocumented `prefix` param (fixed alongside its bracket bug in another follow-up PR), the flag.names_list.Rd/idate.Rd broken-link warnings and dead-code cleanup (fixed in the docs-cleanup PR, not included on this branch since it branches independently from dev), and the %!in% Rd name warning plus a sandbox locale warning, both environment-inherent and unrelated to any source change.
Two bugs, found together while investigating why fixing parFlags()'s
R CMD check error (a separate PR) unmasked a new "checking examples"
ERROR in params.2.fname():
1. Broken logic: nmz <- as.character(substitute(list(...))[-1]) does
NOT recover the argument names. as.character() on a substituted call
deparses each element's VALUE, not its name. Verified directly:
for params.2.fname(aa = 1, cc = 2), this produced nmz = c("1", "2")
(the values, stringified) instead of c("aa", "cc") (the names), so
the function's own documented example, params.2.fname(aa = 1, cc =
2, d = NULL, sep = ".", collapse = "_"), returned "1.1_2.2" instead
of the documented "aa.1_cc.2" - every parameter name in the output
was silently replaced by that parameter's own value.
Fixed by using `names(x)` (x <- list(...) is already computed on the
line above) instead of the substitute/deparse trick.
2. Missing @export: despite having a complete roxygen block (title,
description, params, a worked example) matching every other exported
function's style, and already having a generated man/params.2.fname.Rd,
the function itself was never added to NAMESPACE. Its own example
couldn't even run under R CMD check ("could not find function
params.2.fname"), which is how bug 1 stayed unnoticed - the function
was never reachable by any external caller to begin with. Added @export.
Version bumped 1.2.1 -> 1.2.2 in Development/config.R and DESCRIPTION.
Validation: manually confirmed params.2.fname(aa = 1, cc = 2, d = NULL,
sep = ".", collapse = "_") now returns "aa.1_cc.2" as documented. R CMD
check's "checking examples" ERROR that was previously raised by this
function's own broken example is gone (NAMESPACE now correctly exports
it: export(params.2.fname), man/params.2.fname.Rd was already accurate
and needed no changes). The existing testthat suite still passes 7/7.
Remaining R CMD check findings (parFlags example error - fixed in
another PR in this batch, %!in% Rd name warning, sandbox locale
warning, and doc issues fixed in other independent PRs in this batch)
are pre-existing/environment-inherent and out of scope here.
Co-authored-by: vertesy <5101911+vertesy@users.noreply.github.com>
…e-break-reclassify-clipr # Conflicts: # DESCRIPTION # Development/config.R # NAMESPACE
ReplaceSpecialCharacters() called ReplaceRepeatedDots(x) but discarded its result, so repeated dots were never actually collapsed. parFlags() referenced an undefined `namez`, erroring on every call. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…lassify-clipr Drop dplyr dependency, fix undecided-separator gap, reclassify clipr
match.call() included every supplied argument (prefix, pasteflg, collapsechar), so slicing off the first two elements broke as soon as a control argument was passed or reordered. Extract names directly from the `...` pairlist instead, using each argument's explicit name when given or its deparsed expression otherwise. Co-authored-by: Abel Vertesy <5101911+vertesy@users.noreply.github.com>
An earlier merge on this PR branch removed params.2.fname() entirely under the assumption it was dead code, but it is used in a downstream package. Restore the function together with the original bug fix (names(x) instead of the broken substitute/deparse trick) and its @export tag, man page, and doc/dependency-graph references. Co-authored-by: Abel Vertesy <5101911+vertesy@users.noreply.github.com>
…nd-export Fix params.2.fname(): wrong names + missing export
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8a110d5404
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+11
to
+12
| imports = "methods", | ||
| suggests = "devtools, testthat, clipr", # removed MarkdownHelpers, MarkdownReports, dplyr (no longer used) |
There was a problem hiding this comment.
Problem: Development/config.R still declares version 1.2.5 even though this commit changes dependencies, fixes separator detection, and exports params.2.fname(). Trigger: When this package is distributed over the existing 1.2.5 release, version-based update checks can treat both builds as the same package and leave users on the old implementation. Fix: Increase the version to 1.2.6 in Development/config.R, then regenerate DESCRIPTION.
AGENTS.md reference: AGENTS.md:L44-L44
Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment.
@codex Please Never again try to raise the package version In any of my repositories, in any PR, ever!
There was a problem hiding this comment.
Codex couldn't complete this request. Try again later.
Copilot stopped work on behalf of
vertesy due to an error
August 31, 2026 19:49
Owner
Author
|
I am trying to manually resolve problems in merging the branch to that branch. Please explain it to me in plain and simple language: Why should we update the following code:
to the following verbose expression: Verbose code is the enemy. If this is just fixing some weird edge case that will never occur in real life, then this verbosity isn't worth the expansion. |
Refactor name assignment for 'val' using match.call. This should address Codex's comment.
…gendo.r Fix malformed assignment in parFlags2 and add input validation
Fix discarded return value and undefined variable bugs
|
Codex couldn't complete this request. Try again later. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.