Skip to content

Fix SSP/escape bugs, harden column helpers, add tests, docs & CI#7

Merged
milkway merged 1 commit into
mainfrom
fix/review-bugs-docs-ci
Jun 13, 2026
Merged

Fix SSP/escape bugs, harden column helpers, add tests, docs & CI#7
milkway merged 1 commit into
mainfrom
fix/review-bugs-docs-ci

Conversation

@milkway

@milkway milkway commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Review pass over the package: two real bugs fixed, the options$columns footgun closed, JS-string interpolation hardened, a test suite added, plus documentation/site/CI improvements. Closes #1, #2, #3, #4, #5, #6.

Bug fixes

  • dt2_cols_escape() — both branches of if (escape) returned the same identity renderer, so the parameter was a no-op and the default escape = TRUE rendered raw HTML instead of escaping it (XSS-prone with untrusted data). Now escapes when TRUE, renders raw when FALSE.
  • Server-side processingdt2.js encodes query-string keys with encodeURIComponent (search[value]search%5Bvalue%5D, order[0][column]order%5B0%5D…), but .dt2_parse_ssp_request() only decoded values. Global search and ordering were therefore silently never applied. Keys are now URL-decoded.

Improvements

  • options$columns footgundt2() now injects options$columns from the data when absent (equivalent to dt2.js's client-side derivation), and a new internal .dt2_name_to_idx() centralises name→index resolution and warns instead of silently returning NA when columns are unset or a name is unknown. All name-based helpers use it.
  • Safer JS interpolation.dt2_js_str() (via jsonlite) replaces sprintf("'%s'", x) in the format helpers, fixing invalid JS when values contain quotes.
  • Removed redundant local %||% redefinitions and dead .dt2_resolve_cols(); print.dt2_theme() now shows the class field.

Tests

  • New testthat scaffolding + 43 tests covering the fixed bugs and the new behaviour (escape, SSP key decoding, column resolution/warnings, JS-string safety, theme print). All passing locally.

Docs, site & CI

Notes for the reviewer

  • Behaviour changes in SSP, escaping, and column helpers — worth a full devtools::check() before merge (CI will now run it too).
  • No public functions were removed (CRAN API preserved).

🤖 Generated with Claude Code

Bug fixes
- dt2_cols_escape(): both branches returned the same identity renderer, so
  the parameter was a no-op and the default (escape = TRUE) rendered raw HTML
  instead of escaping it. It now HTML-escapes when TRUE and renders raw when
  FALSE.
- Server-side processing: dt2.js encodes query-string KEYS with
  encodeURIComponent (e.g. "search[value]" -> "search%5Bvalue%5D"), but the
  parser only decoded values. Global search and ordering were therefore never
  applied. Keys are now URL-decoded too.

Improvements
- options$columns footgun: dt2() now injects options$columns from the data when
  absent, and a new .dt2_name_to_idx() centralises name->index resolution and
  warns loudly (instead of silently returning NA) when options$columns is unset
  or a name is unknown. All name-based helpers use it.
- Safer JS interpolation via .dt2_js_str() (jsonlite) replaces
  sprintf("'%s'", x) in the format helpers, fixing broken output when values
  contain quotes.
- Removed redundant local %||% redefinitions and dead .dt2_resolve_cols();
  print.dt2_theme() now shows the class field.

Tests
- Add testthat scaffolding and 43 tests covering the fixed bugs and the new
  behaviour (escape, SSP key decoding, column resolution/warnings, JS-string
  safety, theme print).

Docs, site & CI
- Logo: rebuilt man/figures/logo.svg with explicit dimensions/padding (was
  clipped) and renamed from diagrama-2025-09-11.svg; README uses it. Closes #6
- Real R-CMD-check GitHub Actions workflow; README badge points to it. Closes #1
- Runnable @examples for dt2_order/dt2_search_global/dt2_use_buttons/
  dt2_language. Closes #2
- Document the tidytemplate build dependency in _pkgdown.yml. Closes #3
- Explain the options$columns pattern in the formatting & getting-started
  vignettes. Closes #4
- Reciprocal @Seealso between dt2_buttons() and dt2_use_buttons(). Closes #5
- Grouped pkgdown reference index (was a flat list of 41 functions); README
  version badge 0.1.0 -> 0.1.1, CRAN install instructions, author credits
  aligned to DESCRIPTION (5 authors).
- Migrate to roxygen2 8.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@milkway milkway merged commit 165c1ce into main Jun 13, 2026
5 checks passed
@milkway milkway deleted the fix/review-bugs-docs-ci branch June 13, 2026 23:04
@milkway milkway mentioned this pull request Jun 13, 2026
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.

Add real R-CMD-check CI workflow (badge currently static)

1 participant