feat: Windows port — cross-platform path normalisation and CI matrix#153
Merged
Conversation
- src/path_util.rs: new cross-platform helpers
to_forward_slash() — convert Path to forward-slash string for globset
strip_unc_prefix() — strip UNC prefix from canonicalize() result
file_stem_from_uri() — Url-based file stem with fallback
- src/path_util_tests.rs: tests for all three helpers
- src/rg.rs: add split_rg_fields() — skips C: drive colon so
parse_rg_line and parse_rg_line_with_content_rooted work on Windows
- src/rg_tests.rs: platform-conditional parse_rg_line test
- src/main.rs, src/indexer/apply.rs, src/indexer/cache.rs:
wrap canonicalize() with strip_unc_prefix()
remove bad format!(file://{}) URI construction
- .github/workflows/ci.yml: add windows-latest + choco install
- .github/workflows/release.yml: add x86_64/aarch64-pc-windows-msvc targets
Closes items 1-4 from issue #127.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Direct gs.is_match(rel/name) calls in walkdir_find bypassed the IgnoreMatcher::matches() wrapper that already uses to_forward_slash. On Windows, Path::strip_prefix returns backslash paths which globset rejects. Use crate::path_util::to_forward_slash at all 3 direct gs.is_match call sites in discover.rs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove blank line after doc comment in apply.rs (empty_line_after_doc) - Add #[allow(unused_imports)] to TestDeps cfg(test) re-export in indexer.rs - Drop unused intro_uri binding in references_tests.rs (line 976 shadowed by 981) - Add #[allow(dead_code)] to for_test_with_library (cfg(test), uncalled) - Add #[allow(dead_code)] to sorted_subtype_names in indexer_tests.rs - Add #[allow(dead_code)] to SidecarHandle impl (cfg(not(test)) at call sites) - Add #[allow(dead_code)] to find_java (same reason) - Remove needless &sig_src borrow in it_this_tests.rs:1984 - Replace iter().any() with .contains() in inlay_hints_tests.rs:619 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
macOS lsp_smoke: file_uri() now uses std::fs::canonicalize + Url::from_file_path
to resolve /var -> /private/var symlinks and produce proper file:/// URIs.
On Windows, Url::from_file_path produces file:///C:/... instead of the
broken file://C:\... that format! + display() emitted.
Windows rg tests: normalize backslashes to forward-slash in file path
assertions so 'f.contains("src/Real.kt")' works on both platforms.
Windows workspace_json test: normalize slashes on both sides of the
sources.contains() comparison. workspace.json substitution produces
mixed slashes on Windows (C:\root/sub) which now compare correctly.
ignore_matcher_absolute_path_normalized: mark #[cfg(unix)] since the
test uses Unix-only absolute paths (/workspace) that Windows Path
does not recognize as is_absolute().
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The test previously passed raw tempdir paths (with short 8.3 names on Windows like RUNNER~1, or unresolved /var symlinks on macOS) to the server via KOTLIN_LSP_WORKSPACE_ROOT and initialize rootUri. The server then indexed files using those raw paths, but file_uri() canonicalized to the long form, causing URI mismatches and test failures. Fix: add canonical_root() helper that calls std::fs::canonicalize and strips the Windows \?\ UNC prefix. Use it in LspClient::spawn (env var + current_dir), initialize (rootUri), and file_uri. Now the server and all test assertions use the same canonical path representation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Implements the Windows port tracked in #127 by centralizing path normalization, fixing rg output parsing for drive-letter paths, adding Windows to CI/release matrices, and tightening URL construction in tests.
Changes:
- New
src/path_util.rswithto_forward_slash,strip_unc_prefix, andfile_stem_from_uri, wired into globset matching (rg.rs,discover.rs), canonicalization (main.rs,apply.rs,cache.rs), and qualified-key generation (apply.rs). - New
split_rg_fieldsinsrc/rg.rsthat skips a leadingX:drive colon soparse_rg_lineandparse_rg_line_with_content_rootedwork on Windows; tests updated with#[cfg(windows)]variants. - CI matrix extended with
windows-latest(choco install ripgrep/fd) and release matrix extended withx86_64-pc-windows-msvcandaarch64-pc-windows-msvc(zip packaging); LSP smoke tests now canonicalize roots and build URIs viaUrl::from_file_path.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/path_util.rs / src/path_util_tests.rs | New cross-platform path helpers + tests |
| src/rg.rs | Drive-letter-aware split_rg_fields; globset paths normalized via to_forward_slash |
| src/rg_tests.rs | Platform-specific rg parse inputs and backslash-normalized assertions |
| src/indexer/discover.rs | Direct GlobSet::is_match calls wrapped with to_forward_slash |
| src/indexer/apply.rs | Uses file_stem_from_uri + strip_unc_prefix; drops bad file:// URI concat and the corresponding starts_with check |
| src/indexer/cache.rs | workspace_cache_path strips UNC prefix; save_cache uses file_stem_from_uri |
| src/main.rs | Registers path_util module; --index-only root stripped of UNC prefix |
| src/indexer.rs / src/indexer_tests.rs / src/sidecar.rs | #[allow(dead_code)]/unused_imports to satisfy -D warnings on Windows |
| src/workspace/config_tests.rs | Normalize slashes for cross-platform comparison |
| src/inlay_hints_tests.rs / src/indexer/infer/it_this_tests.rs / src/features/references_tests.rs | Minor clippy/dead-binding cleanups |
| tests/lsp_smoke.rs | Canonicalize roots, strip UNC, use Url::from_file_path for URIs |
| .github/workflows/ci.yml | Tri-OS matrix with platform-specific tool install and -D warnings |
| .github/workflows/release.yml | Windows build targets with .zip packaging |
Comment on lines
34
to
39
| fn classify_source_set(uri: &str, source_paths: &[String]) -> SourceSet { | ||
| for source_path in source_paths { | ||
| if uri.contains(source_path) || uri.starts_with(&format!("file://{}", source_path)) { | ||
| if uri.contains(source_path) { | ||
| return SourceSet::Library; | ||
| } | ||
| } |
| write(root, "IntroContract.kt", intro_src); | ||
| write(root, "LoginContract.kt", login_src); | ||
| let (_, intro_uri) = write(root, "IntroContract.kt", intro_src); | ||
| write(root, "IntroContract.kt", intro_src); |
Comment on lines
+35
to
+42
| - target: x86_64-pc-windows-msvc | ||
| os: windows-latest | ||
| binary: kotlin-lsp.exe | ||
| asset: kotlin-lsp-windows-x86_64 | ||
| - target: aarch64-pc-windows-msvc | ||
| os: windows-latest | ||
| binary: kotlin-lsp.exe | ||
| asset: kotlin-lsp-windows-aarch64 |
- install.sh: bash + set -euo pipefail, wget fallback, KOTLIN_LSP_PREFIX env var, colored output helpers, ~/.local/bin default prefix, PATH hint - install.ps1: new Windows installer; SHA256 verification, sidecar install, arch detection (x86_64/aarch64), PATH update prompt - README: add Windows PowerShell one-liner install command Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- apply.rs: normalize source_path with to_forward_slash before uri.contains() check; on Windows source_paths contain backslashes so substring match silently failed, breaking library classification - references_tests.rs: remove duplicate write(root, IntroContract.kt, ...) left by the unused-binding cleanup - release.yml: add win32-x64 and win32-arm64 vsix packaging; copy Windows .zip artifacts into release/ dir so they appear in sha256sums.txt Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
src/indexer/apply.rs:42
- The previous
classify_source_setcheck accepted either a substring match against the raw source path or afile://<path>prefix match. After normalisingsource_paththroughto_forward_slash, only the substring check remains, and on Windows a configured source path containing backslashes (e.g. fromworkspace.jsonafter<WORKSPACE>substitution on a Windows host, or a user-provided Windows path) will be converted to forward slashes here while theuristring already uses forward slashes encoded byUrl::from_file_path(which also percent-encodes characters like spaces). For Unix this is a pure no-op so behaviour is preserved, but on Windows a source path containing a space or other percent-encodable character will silently no longer match — please verify with a Windows path containing a space, and consider parsinguriviaUrl::to_file_path()and comparing canonicalPathBufs instead of doing a stringcontains.
fn classify_source_set(uri: &str, source_paths: &[String]) -> SourceSet {
for source_path in source_paths {
let normalized = to_forward_slash(std::path::Path::new(source_path));
if uri.contains(&normalized) {
return SourceSet::Library;
}
}
if uri.contains("/src/test/")
Comment on lines
+207
to
+221
| # win32-x64 (no bundled sidecar yet — sidecar not yet cross-compiled for Windows) | ||
| mkdir -p server | ||
| unzip -j "../../artifacts/kotlin-lsp-windows-x86_64/kotlin-lsp-windows-x86_64.zip" \ | ||
| -d server/ | ||
| mv server/kotlin-lsp-windows-x86_64.exe server/kotlin-lsp.exe | ||
| npx @vscode/vsce package --no-dependencies --target win32-x64 -o ../../kotlin-lsp-win32-x64-${GITHUB_REF_NAME}.vsix | ||
| rm -rf server | ||
|
|
||
| # win32-arm64 | ||
| mkdir -p server | ||
| unzip -j "../../artifacts/kotlin-lsp-windows-aarch64/kotlin-lsp-windows-aarch64.zip" \ | ||
| -d server/ | ||
| mv server/kotlin-lsp-windows-aarch64.exe server/kotlin-lsp.exe | ||
| npx @vscode/vsce package --no-dependencies --target win32-arm64 -o ../../kotlin-lsp-win32-arm64-${GITHUB_REF_NAME}.vsix | ||
| rm -rf server |
Comment on lines
+17
to
+18
| REPO="Hessesian/kotlin-lsp" | ||
| INSTALL_DIR="${INSTALL_DIR:-$HOME/.cargo/bin}" | ||
| PREFIX="${KOTLIN_LSP_PREFIX:-$HOME/.local/bin}" |
Comment on lines
+142
to
+147
| $add = Read-Host "Add '$Prefix' to your User PATH now? [y/N]" | ||
| if ($add -ieq "y") { | ||
| $newPath = "$Prefix;$machinePath" | ||
| [System.Environment]::SetEnvironmentVariable("PATH", $newPath, "User") | ||
| Write-Ok "PATH updated — restart your terminal for changes to take effect" | ||
| } |
- extension.js: findServerBinary now tries kotlin-lsp.exe on win32 so the bundled binary in win32 vsix is actually used instead of silently falling back to PATH lookup - install.sh: restore ~/.cargo/bin as default prefix when it exists (preserves existing setups); honour INSTALL_DIR as alias for KOTLIN_LSP_PREFIX for backward compat; _resolve_prefix() encapsulates the priority chain - install.ps1: remove Read-Host prompt (hangs in iwr|iex pipeline); print the manual PATH command instead so non-interactive invocations never block Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
Summary
Implements all critical and high-priority items from #127.
Changes
New:
src/path_util.rsCross-platform path helpers:
to_forward_slash(path)— convertsPathto forward-slash string for globset (no-op on Unix)strip_unc_prefix(path)— strips\\?\UNC prefix fromcanonicalize()result (no-op on Unix)file_stem_from_uri(uri)— URL-based file stem with fallback for driveless URIssrc/rg.rs—split_rg_fieldsAdds Windows-aware
split_rg_fields()that detectsC:\drive-letter prefix and skips that colon before splitting, soparse_rg_lineandparse_rg_line_with_content_rootedwork correctly on Windows.src/indexer/discover.rs— globset forward-slashWires
to_forward_slashinto the 3 directgs.is_match()calls inwalkdir_findthat bypassed theIgnoreMatcher::matches()wrapper.Call sites —
strip_unc_prefix+ URL fixsrc/main.rs,src/indexer/apply.rs,src/indexer/cache.rs: wrapcanonicalize()withstrip_unc_prefix()src/indexer/apply.rs: remove badformat!(\"file://{}\", source_path)URI constructionCI / Release
.github/workflows/ci.yml: addwindows-latestto test matrix withchoco install ripgrep fd.github/workflows/release.yml: addx86_64-pc-windows-msvcandaarch64-pc-windows-msvcrelease targetsReference
Mechanical fixes modelled on qdsfdhvh/kotlin-lsp Windows branch.
Closes #127