docs: use raw URLs in the embedding data/style examples - #1803
Conversation
The `data` and `style` deep-link examples were written with every nested URL percent-encoded, which made them hard to read and copy even though `:` and `/` need no escaping in a query value. Encoding is now shown only where it is actually required, on the REST endpoint that carries its own query string. Also moves "Open remote data" below "Embedding in a page" so the page introduces the iframe before the data parameters it takes.
|
Warning Review limit reached
Next review available in: 1 minute You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughGeoLibre now tests and documents unencoded HTTPS values for ChangesURL Parameter Handling
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔍 Cloudflare PR preview
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Code reviewBugs: None found — this is a docs-only change plus one new test; no runtime logic changed. Security: None found — no user input handling changes. Performance: N/A — docs and a unit test only. Quality:
CLAUDE.md adherence: Doc changes are consistent with the repo's docs conventions ( Section move ( |
🔍 GitHub Pages PR preview
Note GitHub Pages built this preview successfully, but its serving edge returned HTTP 403 when checked. The links may still be propagating. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/user-guide/embedding.md`:
- Line 137: Update the documentation guidance in the embedding URL section to
remove `=` from the characters requiring nested URL encoding, leaving `&`, `+`,
`%`, and `#`. Preserve the REST example’s encoded form because its nested URL
contains `&`.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d85f0d68-7048-4a9b-8f8f-8159adb24367
📒 Files selected for processing (4)
docs/index.mddocs/user-guide/embedding.mddocs/user-guide/layers.mdtests/data-url.test.ts
Publishing a dataset is a common way GeoLibre gets shared, but the docs gave no ready-made badge for a README or catalog entry. Adds Markdown, reStructuredText, and HTML snippets for both the data and project deep links, matching the shields.io style already used elsewhere in the docs.
Code reviewBugs
Security
Performance
Quality
CLAUDE.md
|
- Drop `=` from the list of characters that force encoding in a nested data/style URL. `URLSearchParams` splits each `&`-delimited pair on its first `=` only, so a nested `=` survives verbatim; listing it undercut the point that raw URLs need less encoding than commonly assumed. - Cover that behavior in tests/data-url.test.ts so the documented claim is pinned rather than asserted only in prose.
- Correct the badge caveat. It claimed an unencoded `&` ends a Markdown link at that character; checked against a CommonMark renderer, the link destination survives `&` intact. The real reason to encode is unchanged and already stated: GeoLibre reads the `&` as its own separator.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/user-guide/embedding.md (1)
105-108: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winLimit the REST encoding requirement to characters that require it.
This text says that any endpoint with nested query parameters needs percent-encoding. A URL with only
?category=parkscontains a bare=and parses correctly without encoding. This conflicts with Line 137 and the new parser test.Proposed wording
-An endpoint that takes its own query parameters is the case that does need percent-encoding, so its `&` separators are not read as GeoLibre's own: +If the nested endpoint URL contains `&`, `+`, `%`, or `#`, percent-encode it so the outer URL parser does not alter its value:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/user-guide/embedding.md` around lines 105 - 108, Update the REST API URL guidance near the `data` parameter example to require percent-encoding only for characters that conflict with GeoLibre's outer URL parsing, especially nested `&` separators, rather than for every endpoint query parameter. Clarify that a nested query containing only a parameter such as `?category=parks` can remain unencoded, consistent with the parser behavior and tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@docs/user-guide/embedding.md`:
- Around line 105-108: Update the REST API URL guidance near the `data`
parameter example to require percent-encoding only for characters that conflict
with GeoLibre's outer URL parsing, especially nested `&` separators, rather than
for every endpoint query parameter. Clarify that a nested query containing only
a parameter such as `?category=parks` can remain unencoded, consistent with the
parser behavior and tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c33806c5-85d7-462b-bd58-61b2ad721f22
📒 Files selected for processing (2)
docs/user-guide/embedding.mdtests/data-url.test.ts
|
All checks out: the doc's claims match No inline comments were warranted — I found no bugs, security issues, performance problems, or CLAUDE.md violations in this diff. Code reviewBugs: None found. The doc's encoding guidance ( Security: None found. No behavior/code changes, only documentation and a new pinned test. High confidence. Performance: Not applicable — docs-only change plus a small test addition. Quality: Minor prose nit — the sentence "An endpoint that takes its own query parameters is the case that does need percent-encoding..." (embedding.md:105) reads slightly awkwardly, but it's accurate and not worth blocking on. Low confidence / low severity, not filed inline. CLAUDE.md: No violations. The change stays within Also verified: the |
Summary
dataandstyledeep-link examples in the embedding guide to use plain, unencoded URLs.:and/are legal in a query value, so the percent-encoded spelling was noise that made the examples hard to read and copy. Encoding is now shown only on the REST endpoint example, which carries its own query string and genuinely needs it, and the trailing guidance names the characters that actually break (&,+,%,#) instead of advising encoding everywhere.#open-remote-dataanchor is unchanged, so the inbound links from the layers and styling guides still resolve.docs/index.mdanddocs/user-guide/layers.md, which otherwise disagreed with the embedding guide.data=andurl=deep links. It reuses the shields.io green already used by the badges indocs/getting-started.md. This was added after the PR was opened, so it is a deliberate inclusion rather than a rebase artifact.=behavior. The suite previously covered only theencodeURIComponent-wrapped form, so nothing guarded the form the docs lead with.Test plan
node --import tsx --test tests/data-url.test.tspasses (19/19), covering the raw form and a nested=that survives unencodeddataUrlParametersthat extraction is a plainURLSearchParams.get()with no additional decode step, so raw values round-trip unchanged&does not terminate a Markdown link destination, and corrected the badge caveat that claimed otherwise#open-remote-datastill landSummary by CodeRabbit
Documentation
Tests