Skip to content

Tester/windows path fix - #36

Merged
fabriceHategekimana merged 11 commits into
mainfrom
tester/windows-path-fix
Oct 19, 2026
Merged

fabriceHategekimana merged 11 commits into
mainfrom
tester/windows-path-fix

Conversation

@CyuHat

@CyuHat CyuHat commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

Three commands (typr pkg install, typr pkgdown, typr document)
build an R command as a string and drop the current directory into it.
On Windows that path has backslashes, R reads \U as a unicode escape,
and the command dies before running. This PR adds a small r_path()
helper that converts backslashes to forward slashes — valid in R on
Windows too — at those three call sites. Unix paths have no
backslashes, so nothing changes there.

Checks

  • cargo test --workspace passes locally — with one caveat:
    format_r::tests::styles_messy_code_when_available fails, but it
    fails on a clean checkout too (verified by stashing). It's the
    known air formatter version issue, unrelated to this change.
    The 4 new tests for r_path() pass.
  • cargo fmt --all applied
  • typr case run shows no REGRESS; not ticked on purpose.
    My Windows run shows 14 REGRESS, and I dug into every one (full
    write-up below). None come from this PR: 8 are my local air
    version not formatting output (whitespace-only diffs, verified
    identical modulo whitespace), 2 are case bundles that are
    internally inconsistent and should fail on CI too (0066/0067),
    and 4 are the case runner's @run grep tripping on Windows CRLF
    line endings in R's output. A Linux CI run should be green
    except possibly 0066/0067.

If this PR touches the parser or the type checker

It doesn't, only the CLI. Unit tests added instead of a cases/ entry:
the bug is in how the CLI shells out to R, not in the language, so a
catalogue case (which replays check/build/run on .ty files)
can't exercise it.

Validation on Windows

I installed the dev build and ran the three commands in a real
project. typr pkg install and typr document now work since the \U
error is gone. typr pkgdown gets past the path parsing and fails
later for an unrelated reason (Pandoc not installed on my machine as
an environment prerequisite, not this fix).

Testing surfaced three follow-up issues, none caused by this PR,
which I'll file separately:

  1. typr init reported devtools as installed when the project's renv couldn't load it — the check seems to look at the global R library instead of the project library.
  2. devtools::install_local() is deprecated (since devtools 2.5.0, pointing to pak::pak("local::...")) and now requires the remotes package, which a fresh renv project doesn't have. The install still completed.
  3. typr pkgdown needs Pandoc on PATH and fails deep inside a callr subprocess when it's missing. An early check with a clear message would be nicer.

fabriceHategekimana and others added 11 commits October 5, 2026 16:59
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
devtools::install_local(), pkgdown::build_site() and
devtools::document() interpolated the current directory into a
single-quoted R string. On Windows, R's parser reads backslashes as
escape sequences ('\U' starts a unicode escape), aborting the command
with "'\U' used without hex digits in character string".

Add an r_path() helper that converts backslashes to forward slashes —
valid path separators in R on every platform, Windows included — and
use it at the three call sites. No-op on Unix paths.

Unit tests cover the Windows conversion, the Unix no-op, the '.'
fallback, and the generated R command containing no backslash.

@fabriceHategekimana fabriceHategekimana left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The change is good. It replace the parts needed for windows paths.

@fabriceHategekimana
fabriceHategekimana merged commit c3f7225 into main Oct 19, 2026
10 of 11 checks passed
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.

2 participants