fix(viewer): percent-decode link targets in _extract_links - #203
Open
oierreaemme wants to merge 1 commit into
Open
fix(viewer): percent-decode link targets in _extract_links#203oierreaemme wants to merge 1 commit into
oierreaemme wants to merge 1 commit into
Conversation
Per SPEC section 5, links to file names containing spaces are percent-encoded (e.g. `[report](weekly%20report.md)`). _extract_links resolved the raw target without decoding, producing edge ids like `notes/weekly%20report` that never match the concept ids derived from on-disk paths (`notes/weekly report`), so every such edge was dropped: bundles whose file names contain spaces render with zero edges. Decode the captured target with urllib.parse.unquote before resolving. Decoding happens before the scheme/absolute-path checks so encoded forms of external or absolute targets are still skipped. Scoped fix: absolute bundle-relative links remain unsupported here (tracked separately in GoogleCloudPlatform#48). Adds a regression test. Fixes GoogleCloudPlatform#200
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Author
|
@googlebot I signed it! |
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.
Fixes #200
Problem
Per SPEC §5, links to file names containing spaces are percent-encoded (e.g.
[report](weekly%20report.md))._extract_linksinokf/src/reference_agent/viewer/generator.pyresolves the raw regex capture without decoding it, so the emitted edge id isnotes/weekly%20reportwhile the concept node id derived from the on-disk path isnotes/weekly report. The edge never matches a node and is dropped: a bundle whose file names contain spaces renders with zero edges even though every link is spec-conformant.Fix
Decode the captured target with
urllib.parse.unquotebefore resolving it. Decoding happens before the scheme/absolute-path guards, so percent-encoded forms of external (https%3A%2F%2F…) or absolute (%2F…) targets are still skipped. Targets containing a literal%not followed by two hex digits are left untouched byunquote.Scope
Deliberately minimal:
/Note.md) remain unsupported by the viewer — that is visualize: 0 edges for spec-recommended absolute links and CommonMark link titles #48 (PRs fix: viewer absolute links + spec alignment across codebase (fixes #48) #66/okf: fix visualize edges for absolute and titled cross-links #184);_LINK_REstill rejects literal unencoded spaces, which is consistent with markdown link syntax and with SPEC §5's encoding recommendation;Testing
okf/tests/test_viewer.py: 7 passed (6 existing + new regression testtest_percent_encoded_links_become_edges, which fails without the fix).