Fix file-link parser capturing trailing sentence period for .md paths (Closes #11477)#11535
Draft
cbeaulieu-gt wants to merge 2 commits into
Draft
Conversation
Terminal output that ends with a path-then-period (e.g. "Drafted at
docs/spec.md.") was not recognized as a clickable markdown file:
clicking "Open in Warp" opened it in a generic text pane with no
Editor/Viewer toggle.
Two layers contributed:
1. On POSIX, the grid-based file-link tokenizer's SUFFIXES_TO_REMOVE
only stripped '@' (the ls symlink indicator), so "foo.md." never
produced a "foo.md" fallback candidate when the filesystem lookup
for the literal name failed.
2. On Windows, the NT kernel silently normalizes trailing periods in
path lookups, so fs::metadata("foo.md.") resolves to foo.md and
succeeds — but the returned PathBuf carries the literal trailing
dot through to is_markdown_file, where Path::extension() returns
Some("") and rejects the file.
This change addresses both layers: '.' is added to SUFFIXES_TO_REMOVE,
and absolute_path_if_valid strips a trailing '.' from the returned
PathBuf on Windows so downstream classifiers see the canonical form.
The parallel text-path tokenizer in app/src/util/link_detection.rs
already handles trailing periods (lines 254-261); this brings the
grid path to parity and adds the Windows-side normalization that
neither path was doing.
Fixes warpdotdev#11477
The earlier fix (commit 654be7f) made foo.md. resolve correctly on both platforms, but on Windows the user-visible link highlight still included the trailing '.'. On POSIX, the literal lookup fails and the SUFFIXES_TO_REMOVE fallback at link_detection.rs:561 shrinks the highlight end_point by suffix.len(). On Windows, the NT kernel normalises the trailing '.' transparently, the literal lookup succeeds on the first try, the fallback never runs, and the highlight is never shrunk. This change adds a cfg(target_os = "windows")-gated branch in the success arm of the literal lookup: when the captured token ends in '.' and is not a verbatim long path (starting with \?\), retry the stripped form and prefer it (with the shrunk end_point) when it resolves. Mirrors the established SUFFIXES_TO_REMOVE pattern; POSIX is bit-for-bit unchanged. The verbatim long-path guard avoids the one network-attached-storage edge case where Win32 normalisation is bypassed and a Linux/POSIX filesystem could legitimately serve distinct foo.md and foo.md. files. Guard verified by code inspection, no hermetic test path for verbatim UNC paths (requires elevated privileges not available in CI environments). Extends warpdotdev#11477.
github-actions
Bot
added
the
external-contributor
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
1 participant
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
Fixes #11477 — the file-link parser captured a trailing sentence period as part of
.mdpaths, breaking the markdown viewer activation and making the visible link highlight inconsistent across platforms.Two commits, both scoped to
app/src/terminal/view/link_detection.rs:654be7f1Strip trailing '.' from file-link candidate paths — adds.toSUFFIXES_TO_REMOVEsofoo.md.resolves tofoo.mdon the fallback path and the markdown classifier sees the correct extension. Addresses AC Search history by more than just command #1 (Editor/Viewer toggle appears).6ca44367Match POSIX highlight-range behaviour on Windows for trailing '.' — addresses AC Real-time collaboration #2 (highlight-range parity). On POSIX, the fallback path shrinksend_pointbysuffix.len()when the stripped form resolves; on Windows that branch is never reached because the NT kernel transparently normalizesfoo.md.→foo.mdand the literal lookup at line 542 succeeds first. The fix adds a#[cfg(target_os = "windows")]-gated branch in the literal-lookup success arm: when the captured token ends in., also try the stripped form and prefer it (with shrunkend_point) when both resolve.Cross-platform parity
POSIX object code stays bit-for-bit identical — the new branch and its dependencies are excluded from Linux/macOS binaries by the
cfggate. The change is Windows-only by construction.The
\\?\long-path prefix guard (!path.starts_with("\\?\\")) is included so the change is correct in the edge case where a Linux SMB server serves distinctfoo.mdandfoo.md.files via a long-path-prefixed mount — that syntax bypasses Win32 normalization, so we must not conflate the two in that one case.Test plan
compute_valid_paths_windows_highlight_range_parityadded; fulllink_detectionsuite (12 tests) greencargo clippy --all-targets --tests -- -D warningswarp-ossrelease build: case 2 (echo "Drafted at C:/Users/chris/warp-md-test.md.") opens with the Editor↔Viewer toggle present and the visible underline excludes the trailing.ready-to-implement(or equivalent) label applied before mergeScope decision
This PR is scoped to the trailing-
.case from #11477's acceptance criteria. The broader closing-punctuation set (,;:!?)]}>"') discussed in the original issue body is intentionally not included here; if maintainers want that expansion it should be a separate scope conversation / follow-up issue rather than folded in.🤖 Generated by Claude Code on behalf of @cbeaulieu-gt