fix(pampa): normalize resolve_path output to forward slashes#340
Merged
Conversation
resolve_path joined the script dir with Path::join and returned the native string, so on Windows extension authors saw backslash paths (C:\ext\dir\data.json). The output feeds io.open/dofile/mediabag.fetch, which tolerate backslashes natively, so this was cosmetic on native — but it diverged from the pathWithForwardSlashes convention the rest of the tree already follows (quarto-util::to_forward_slashes, established by bd-vxl8) and left the path-comparison tests platform-dependent. normalize_path and the rooted/no-script-dir return paths now go through to_forward_slashes, so resolve_path returns forward slashes on every platform — one invariant, no cfg-gated test expectations. Also swap the is_absolute() gate for quarto_util::is_rooted(). is_absolute returns false on wasm32 for /project VFS paths and on Windows for drive-less rooted paths, which would let such a path fall through and wrongly join onto the script dir. No current caller passes an absolute path here, so this is behavior-neutral today but closes the latent WASM mis-join and matches the is_rooted convention bd-vxl8 set in the io/dofile WASM paths.
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
2 participants
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.
On Windows,
quarto.utils.resolve_pathjoins the script directory with the requested path viaPath::join, so the Lua-visible result carries native backslash separators (C:\ext\dir\data.json). The native render path is unaffected — the consumers (io.open,fs::readviamediabag.fetch,dofile) all accept backslashes on Windows — but the cosmetic separator difference breaks ~10 path assertions inquarto_api.rsthat hardcode forward slashes.Investigating the separator handling surfaced a second, latent bug:
resolve_pathgated onPath::is_absolute(). Onwasm32(and on Windows for a drive-less rooted path like/project/x),is_absolute()returnsfalse, so a rooted VFS path would fall through and wrongly join onto the script dir. bd-vxl8 already sweptio_wasm.rs/dofile_wasm.rsfromis_absolute/starts_with('/')toquarto_util::is_rooted;resolve_pathwas missed. No current caller passes a rooted path here, so it is latent today — but a real convention divergence.Fix
Normalize on the production side rather than cfg-gating the test expectations:
is_absolute()forquarto_util::is_rooted()(=has_root(), correct on both native and WASM).quarto_util::to_forward_slashes, establishing a single invariant:resolve_pathalways returns forward slashes.This matches the boundary-normalization precedent set by bd-vxl8 and quarto-cli's
pathWithForwardSlashesconvention, and lets the tests assert forward slashes on all platforms with no#[cfg(windows)]duplication.The behavior change is low-risk: output goes from backslash to forward-slash on Windows, and all known consumers accept forward slashes there. WASM already produced forward slashes and is unaffected. The
is_rootedswap is behavior-neutral today and forward-compatible for a future WASM caller passing a/project/...path.Two new tests pin the contract:
resolve_pathreturns forward slashes when the pushed script dir itself carries backslashes (the realfilter.rs/shortcode.rsinput on Windows), and a rooted input is returned as-is and never joined onto an active script dir.Strand: bd-picv