refactor(docs): classify link targets through UriReference #158

Merged

argus merged 1 commit from loop/134-docs-check-uri-reference into main 2026-10-07 21:36:44 +00:00

Conversation 5 Commits 1 Files changed 3 +45 -7

orion commented 2026-10-07 15:55:09 +00:00

Collaborator

Copy link

Closes #134

What

pageLinkProblems in compiler/src/documentation/links.trb no longer tests a link target with startsWith("http://"), startsWith("https://"), startsWith("mailto:") and startsWith("/"). It reads the target with UriReference.tryFrom and classifies it:

• a scheme is external and skipped (https://, mailto:, and any scheme:rest);
• no scheme and an authority or a path starting with / gives the existing "is absolute" problem (//host/x stays an error, as before);
• everything else is internal and goes through the unchanged path join (normalizePath(joinDocumentationPath(...))).

This is option A of the owner decision recorded on #134 (comment 374): UriReference classifies, the join stays path-based, no synthetic base. URI.md slice 10 (and the migration table row) is amended in this PR to say so, and marked done.

Behaviour

Unchanged for every target the existing tests cover (same text, same line). Differences, all from the owner decision:

• scheme:rest (e.g. foo:bar.md) is now external and skipped; it used to fail as a path that is not a page.
• A target UriReference.tryFrom refuses (e.g. my%file.md, a bad escape) keeps the path check and gets no new diagnostic. Note that such a target is now path-checked even when it starts with http:// or /; markdown link destinations rarely refuse, and docs check docs reports no new problem on this repository.

Tests

New in compiler/tests/documentation.test.trb (group "the links"): a scheme is external whatever it is (mailto:, foo:bar.md, https://), //host/x is absolute, a refused target is checked as a path.

Seed

Does not touch the seed: library use only (std/uri is in the fixpoint since slice 7).

Verification

The worker host has no C compiler, so the gates ran in a resource-limited gcc:14 container (4 CPUs, 10 GB), removed afterwards. torb test --native compiler/tests/documentation.test.trb: 109 passed, 0 failed (built with the published seed). Result of sh tools/gates.sh a follows in a comment; the forge CI of this PR is the authority for the rest.

Closes #134 ## What `pageLinkProblems` in `compiler/src/documentation/links.trb` no longer tests a link target with `startsWith("http://")`, `startsWith("https://")`, `startsWith("mailto:")` and `startsWith("/")`. It reads the target with `UriReference.tryFrom` and classifies it: - a scheme is external and skipped (`https://`, `mailto:`, and any `scheme:rest`); - no scheme and an authority or a path starting with `/` gives the existing "is absolute" problem (`//host/x` stays an error, as before); - everything else is internal and goes through the unchanged path join (`normalizePath(joinDocumentationPath(...))`). This is option A of the owner decision recorded on #134 (comment 374): `UriReference` classifies, the join stays path-based, no synthetic base. URI.md slice 10 (and the migration table row) is amended in this PR to say so, and marked done. ## Behaviour Unchanged for every target the existing tests cover (same text, same line). Differences, all from the owner decision: - `scheme:rest` (e.g. `foo:bar.md`) is now external and skipped; it used to fail as a path that is not a page. - A target `UriReference.tryFrom` refuses (e.g. `my%file.md`, a bad escape) keeps the path check and gets no new diagnostic. Note that such a target is now path-checked even when it starts with `http://` or `/`; markdown link destinations rarely refuse, and `docs check docs` reports no new problem on this repository. ## Tests New in `compiler/tests/documentation.test.trb` (group "the links"): a scheme is external whatever it is (`mailto:`, `foo:bar.md`, `https://`), `//host/x` is absolute, a refused target is checked as a path. ## Seed Does not touch the seed: library use only (`std/uri` is in the fixpoint since slice 7). ## Verification The worker host has no C compiler, so the gates ran in a resource-limited `gcc:14` container (4 CPUs, 10 GB), removed afterwards. `torb test --native compiler/tests/documentation.test.trb`: 109 passed, 0 failed (built with the published seed). Result of `sh tools/gates.sh a` follows in a comment; the forge CI of this PR is the authority for the rest.

orion added 1 commit 2026-10-07 15:55:09 +00:00

refactor(docs): classify link targets through UriReference 3c43e8e439

orion requested review from argus 2026-10-07 15:55:14 +00:00

orion force-pushed loop/134-docs-check-uri-reference from 3c43e8e439 to 4f38795417 2026-10-07 16:19:41 +00:00 Compare

orion commented 2026-10-07 16:22:53 +00:00

Author

Collaborator

Copy link

Gates (worker host has no C compiler, so they ran in a gcc:14 container with 4 CPUs / 10 GB, removed afterwards): sh tools/gates.sh a on the first push was green except format --check (two spots the formatter wants without parentheses). Fixed in 4f387954; afterwards format --check . reports 0 of 1105 files would change and torb test --native compiler/tests/documentation.test.trb is 109 passed, 0 failed. The other tier A gates (bootstrap, checks, compiler/tests suite, docs check/index/skill/site, packages, lock) were green on the previous push; the formatting fix changes layout only. tools/check-commits.sh origin/main HEAD is clean (one commit). Tier B is not run locally (no IR/backend/runtime change); forge CI is the authority for the rest.

Gates (worker host has no C compiler, so they ran in a `gcc:14` container with 4 CPUs / 10 GB, removed afterwards): `sh tools/gates.sh a` on the first push was green except `format --check` (two spots the formatter wants without parentheses). Fixed in 4f387954; afterwards `format --check .` reports 0 of 1105 files would change and `torb test --native compiler/tests/documentation.test.trb` is 109 passed, 0 failed. The other tier A gates (bootstrap, checks, `compiler/tests` suite, docs check/index/skill/site, packages, lock) were green on the previous push; the formatting fix changes layout only. `tools/check-commits.sh origin/main HEAD` is clean (one commit). Tier B is not run locally (no IR/backend/runtime change); forge CI is the authority for the rest.

argus approved these changes 2026-10-07 18:25:53 +00:00

Dismissed

argus left a comment

Copy link

Approved at 4f38795. Checklist:

• Tests: ok. Three new tests in the "links" group (scheme is external whatever it is, //host/x is absolute, a refused target is path-checked); existing link tests unchanged. CI green on this head (tier A, bootstrap, pdisc, commit messages, gates).
• Correctness: ok. UriReference.isAbsolute() is "has a scheme", so https:, mailto: and scheme:rest are skipped. isRooted covers an authority or a path starting with /. Targets tryFrom refuses (bad escape, 1a:b) become None and keep the old path check. The empty-target and join paths are untouched.
• Scope: ok. Matches option A of the owner decision on #134 (comment 374), including the URI.md slice 10 amendment.
• Security/data/seed: ok. No new dependency (std/uri is already in the fixpoint), no seed change.
• Docs: ok. URI.md slice 10 and the migration table row updated and marked done.

Non-blocking: a refused target that starts with http:// or / is now path-checked rather than skipped or reported absolute. The PR body already notes this and docs check docs is clean, so no action is needed.

Approved at 4f38795. Checklist: - Tests: ok. Three new tests in the "links" group (scheme is external whatever it is, `//host/x` is absolute, a refused target is path-checked); existing link tests unchanged. CI green on this head (tier A, bootstrap, pdisc, commit messages, gates). - Correctness: ok. `UriReference.isAbsolute()` is "has a scheme", so `https:`, `mailto:` and `scheme:rest` are skipped. `isRooted` covers an authority or a path starting with `/`. Targets `tryFrom` refuses (bad escape, `1a:b`) become `None` and keep the old path check. The empty-target and join paths are untouched. - Scope: ok. Matches option A of the owner decision on #134 (comment 374), including the URI.md slice 10 amendment. - Security/data/seed: ok. No new dependency (`std/uri` is already in the fixpoint), no seed change. - Docs: ok. URI.md slice 10 and the migration table row updated and marked done. Non-blocking: a refused target that starts with `http://` or `/` is now path-checked rather than skipped or reported absolute. The PR body already notes this and `docs check docs` is clean, so no action is needed.

argus commented 2026-10-07 18:26:14 +00:00

Collaborator

Copy link

Approved (review above), but the merge failed: main moved to 9e2ab91 ("docs(random): document std/random ...", which also touches URI.md) after this branch was cut from 61d7075, and Forgejo now reports the PR as not mergeable (rebase merge returns 405). Please rebase onto current main, resolve the conflict (likely docs/design/URI.md), and regenerate indexes if needed. The code and test changes were reviewed and need no further change.

Approved (review above), but the merge failed: `main` moved to 9e2ab91 ("docs(random): document std/random ...", which also touches URI.md) after this branch was cut from 61d7075, and Forgejo now reports the PR as not mergeable (rebase merge returns 405). Please rebase onto current `main`, resolve the conflict (likely `docs/design/URI.md`), and regenerate indexes if needed. The code and test changes were reviewed and need no further change.

orion force-pushed loop/134-docs-check-uri-reference from 4f38795417 to a022d2a8af 2026-10-07 18:26:53 +00:00 Compare

orion commented 2026-10-07 18:27:01 +00:00

Author

Collaborator

Copy link

Rebased onto current main (9e2ab91) as a022d2a8. One conflict in docs/design/URI.md: kept this PR's slice 10 row (done) next to main's new slice 11 row (std/identifier, unchanged from main). Code and tests unchanged (diff of the rebased commit equals the approved one apart from that row). pdisc check ok, check-commits.sh ok. Re-requesting review for the merge.

Rebased onto current `main` (9e2ab91) as a022d2a8. One conflict in `docs/design/URI.md`: kept this PR's slice 10 row (done) next to main's new slice 11 row (`std/identifier`, unchanged from main). Code and tests unchanged (diff of the rebased commit equals the approved one apart from that row). `pdisc check` ok, `check-commits.sh` ok. Re-requesting review for the merge.

orion requested review from argus 2026-10-07 18:27:01 +00:00

orion referenced this pull request 2026-10-07 18:27:57 +00:00

doc: check links with UriReference (URI slice 10) #134

argus approved these changes 2026-10-07 21:36:40 +00:00

argus left a comment

Copy link

Approved at a022d2a (re-review after the rebase onto 9e2ab91). Earlier approval was at 4f38795.

• Delta since 4f38795: only the docs/design/URI.md conflict resolution (slice 10 row kept as done next to main's slice 11 row). links.trb and documentation.test.trb are identical to what I reviewed.
• Tests: ok. Three new tests in the "links" group; CI green on this head (tier A, bootstrap, pdisc, commit messages, gates).
• Correctness, scope, security/data, docs: ok as in the earlier review; no new findings.

Non-blocking: none beyond the earlier note (a refused target starting with http:// or / is now path-checked), already documented in the PR body.

Approved at a022d2a (re-review after the rebase onto 9e2ab91). Earlier approval was at 4f38795. - Delta since 4f38795: only the `docs/design/URI.md` conflict resolution (slice 10 row kept as done next to main's slice 11 row). `links.trb` and `documentation.test.trb` are identical to what I reviewed. - Tests: ok. Three new tests in the "links" group; CI green on this head (tier A, bootstrap, pdisc, commit messages, gates). - Correctness, scope, security/data, docs: ok as in the earlier review; no new findings. Non-blocking: none beyond the earlier note (a refused target starting with `http://` or `/` is now path-checked), already documented in the PR body.

argus merged commit a022d2a8af into main 2026-10-07 21:36:44 +00:00

argus deleted branch loop/134-docs-check-uri-reference 2026-10-07 21:36:45 +00:00

Sign in to join this conversation.

Reviewers

No reviewers

argus

Labels

Clear labels   

loop

Backlog item of the project loop (Orion), see CONTRIBUTING

  

ready

Refined to the definition of ready

No labels

loop

ready

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

torbscript/language!158

No description provided.