Fix discarded return value and undefined variable bugs

[vertesy]

Summary

Fixes 2 bugs flagged during the recent code-annotation pass, both in R/Stringendo.R.

Why

• ReplaceSpecialCharacters() called ReplaceRepeatedDots(x) but discarded the result — repeated dots were silently never collapsed. Now assigns it back to x.
• parFlags() referenced namez, which was never defined — every call errored. Restored the namez <- as.character(as.list(match.call())[-(1:2)]) line that its successor parFlags2() already has.

Verified both fixes interactively via devtools::load_all(); no testthat suite exists in this repo to run.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0c2a5398ea

ℹ️ 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".

[chatgpt-codex-connector]

Extract parFlags names only from the dots

Problem: match.call() contains every supplied argument, but removing its first two elements only works when prefix is explicitly first and no control argument follows. Trigger: parFlags(pearson = TRUE) drops pearson, while calls that set documented arguments such as collapsechar also include that argument in namez, causing wrong names or a length mismatch at names(val) <- namez. Fix: Capture expressions from ... directly instead of slicing the complete call.

AGENTS.md reference: AGENTS.md:L25-L29

Useful? React with 👍 / 👎.

[vertesy]

@claude

1
Could you please address the following comment from Codex?

"Problem: match.call() contains every supplied argument, but removing its first two elements only works when prefix is explicitly first and no control argument follows. Trigger: parFlags(pearson = TRUE) drops pearson, while calls that set documented arguments such as collapsechar also include that argument in namez, causing wrong names or a length mismatch at names(val) <- namez. Fix: Capture expressions from ... directly instead of slicing the complete call."

2
Would you please resolve the branch conflicts so that I can merge it directly?

[claude]

Claude finished @vertesy's task in 3m 52s —— View job

---

Summary

• Gather context (PR diff, repo guidelines)
• Address Codex feedback on parFlags() namez extraction
• Investigate branch conflicts with dev
• Push changes

1. Codex feedback on parFlags() — fixed

Pushed b9f616c to this branch. R/Stringendo.R:1580 now extracts names only from the ... pairlist instead of slicing the whole match.call():

dotz <- match.call(expand.dots = FALSE)$"..."
namez <- vapply(seq_along(dotz), function(i) {
  nm <- names(dotz)[i]
  if (!is.null(nm) && nzchar(nm)) nm else deparse(dotz[[i]])
}, character(1))
names(val) <- namez

This fixes both problems Codex raised: prefix/pasteflg/collapsechar can no longer leak into namez regardless of argument order, and a named dot-argument like parFlags(pearson = TRUE) now correctly yields "pearson" instead of being dropped (unnamed/positional args like the documented parFlags(prefix = "MyPlot", pearson, filtered, normalized) usage still resolve via deparse() of the symbol, as before).

2. Branch conflicts — could not resolve, here's why

Two blockers:

• Tooling: this environment's sandbox denies git fetch, git merge, and gh api/gh pr view for me (all require an approval step that never arrives in this non-interactive context), so I can't fetch full history or actually perform/push a merge here.
• More importantly — dev currently has broken code that isn't about conflict markers. Comparing trees directly (git diff origin/dev HEAD -- R/Stringendo.R), the tip of dev (commit 99e9392) has this in parFlags2():

  val <- c(...)
  
  
  
  
  <- as.character(as.list(match.call())[-(1:2)])
  names(val) <- namez

  The namez <- assignment lost its left-hand variable name — that's a syntax error, so R/Stringendo.R won't currently source()/load from dev at all. That's unrelated to this PR but will block any merge regardless of conflict resolution, and should probably be fixed on dev directly first.
• Separately, dev has already picked up its own (partial) fix for the exact parFlags() bug this PR addresses (names(val) <- as.character(match.call(expand.dots = FALSE)[["..."]])), plus unrelated changes to ReplaceSpecialCharacters(), parsepvalue(), countDotOrUnderscoreSeparated(), toCamelCase/toUnderscoreSeparated, ParseFullFilePath(), and flag.names_list() — so this won't be a trivial fast-forward even once dev is fixed.

I'd recommend fixing the broken parFlags2() line on dev first, then either using GitHub's "Update branch" button on this PR or asking me again — with git fetch/git merge permitted — to reconcile the overlapping fixes (in particular, whichever parFlags()/parFlags2() version should win).
· Branch: fix/discarded-and-missing-var-bugs

[vertesy]

@claude

I resolved one of the conflicts. The other conflict is remaining, and I need your evaluation on that.

<<<<<<< fix/discarded-and-missing-var-bugs
  dotz <- match.call(expand.dots = FALSE)$"..."
  namez <- vapply(seq_along(dotz), function(i) {
    nm <- names(dotz)[i]
    if (!is.null(nm) && nzchar(nm)) nm else deparse(dotz[[i]])
  }, character(1))
  names(val) <- namez
=======
  names(val) <- as.character(match.call(expand.dots = FALSE)[["..."]])
>>>>>>> dev

It looks like the current change is a very verbose way to guard against some errors that I have never experienced. Does it even worth it? I'm not sure if I fully understand what the current change is suggesting and why the incoming change is not sufficient. I understand that we fixed this match call, which Codex declared as being problematic. But just looking at the resolution, I don't want that, because I just don't want to turn 1 line into more or less works into 6 lines.

Verbose code is our enemy. Remember that.

[claude]

Claude finished @vertesy's task in 2m 8s —— View job

---

Tasks

• Gather context (PR diff, conflict, Codex feedback)
• Evaluate the two parFlags() conflict resolutions
• Post recommendation

Evaluation: which side to keep

Keep your branch's version (top side), not dev's one-liner. They don't actually fix the same amount of bug — dev's line still mislabels the exact case Codex called out, it just no longer crashes on it.

Walk through parFlags(pearson = TRUE) (a named dot-arg, no prefix supplied) through both:

• dev: as.character(match.call(expand.dots = FALSE)[["..."]]) — this correctly isolates just the ... pairlist (so collapsechar/prefix/pasteflg never leak in, which was the main crash Codex reported). But as.character() on a pairlist converts each value, not its name. So for pearson = TRUE, this returns "TRUE" — the flag gets silently labeled "TRUE" instead of "pearson". No crash, but wrong data.
• Your branch's vapply: checks names(dotz) per element first, falling back to deparse() only for unnamed (positional/symbol) args. For pearson = TRUE it correctly returns "pearson". For the documented bare-symbol usage (parFlags(prefix = "MyPlot", pearson, filtered, normalized)) it behaves identically to dev's version (deparse(quote(pearson)) → "pearson").

So the extra lines aren't defensive padding against a hypothetical — they're the only way to distinguish "dot-arg supplied as name = value" from "dot-arg supplied as bare symbol," which R has no single built-in idiom for (names() on a pairlist is NULL when nothing is named, and only becomes a full vector with "" placeholders once something is named — that asymmetry is exactly why a naive ifelse(nzchar(names(dotz)), ...) one-liner breaks on the all-positional case, i.e. the documented usage).

That said, your instinct is right that this is a low-stakes edge case in practice: parFlags() is .Deprecated("parFlags2"), and every documented/example usage passes bare symbols, never name = value. If named dot-args are never actually used in this codebase, dev's shorter line is a defensible "good enough" fix for the crash, and the mislabeling would never trigger. It's a real tradeoff between "handles an edge case Codex explicitly named" vs. "1 line instead of 6."

Recommendation: keep your branch's version in the conflict (resolve in favor of fix/discarded-and-missing-var-bugs) — it closes the exact gap Codex flagged rather than only half of it, and since parFlags() is deprecated anyway, this code won't need touching again.

Not implementing this in your local merge myself since it's an in-progress conflict resolution in your checkout, not something visible in this repo's state — let me know if you'd like me to instead push a resolution directly to this PR branch.
· Branch: fix/discarded-and-missing-var-bugs