From 2a8985334ddf0dd81239df091942c9b1581deadc Mon Sep 17 00:00:00 2001 From: Patrick <320190286+muellerei@users.noreply.github.com> Date: Wed, 16 Sep 2026 19:55:44 +0200 Subject: [PATCH 1/3] Add a link checker, because a broken reference still looks like a link Every relative link between the Markdown files claims a file and a heading exist. Nothing verified that, so a rename broke them silently. The anchor rule is the part worth writing down: GitHub drops punctuation before turning spaces into hyphens, so an em dash in a heading leaves both its surrounding spaces behind and the anchor gets two hyphens, not one. A first version collapsed them and reported an intact link as broken. Generated and untracked trees are skipped. An earlier version read them and came back with a broken link inside a gitignored directory -- a failure no contributor could reproduce, in a file they cannot see. Run over the repository: 5 files, 0 broken links. Verified it can fail by appending a dangling file reference and a dangling anchor to README.md: both reported, exit 1. --- scripts/check-links.py | 91 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 scripts/check-links.py diff --git a/scripts/check-links.py b/scripts/check-links.py new file mode 100644 index 0000000..389b93a --- /dev/null +++ b/scripts/check-links.py @@ -0,0 +1,91 @@ +#!/usr/bin/env python3 +"""Check relative links and heading anchors across the Markdown files. + +Every link between the documents here is a claim that a file and a heading +exist. Nothing verifies that, and a rename breaks it silently -- the link still +looks like a link. + +Run it over the repository root: + + python3 scripts/check-links.py . + +External links (http, https, mailto) are not checked: that needs the network, +and a 404 somewhere else is not the same class of error as a reference to our +own file that no longer resolves. +""" +import pathlib +import re +import sys + +# Generated or untracked trees hold Markdown too, and a checker that reads them +# reports failures nobody else can reproduce: `local/` is gitignored and exists +# only on one machine, `.pytest_cache/README.md` is written by pytest. Both were +# in scope in an earlier version, which is how a run over the repository came +# back with a broken link that no contributor could have seen. +SKIP_DIRS = {".git", "local", ".pytest_cache", ".venv", "venv", "node_modules"} + +EXTERNAL = ("http://", "https://", "mailto:") + +LINK_RE = re.compile(r"\[([^\]]+)\]\(([^)]+)\)") +HEADING_RE = re.compile(r"^#{1,6}\s+(.*)$") + + +def anchor(heading): + """Slug a heading the way GitHub does. + + The rule that is easy to get wrong: punctuation is dropped *before* spaces + are turned into hyphens, and the surrounding spaces stay. So an em dash in + "007 - Write the rules down" leaves two spaces behind and the anchor gets + **two** hyphens, not one. A first version collapsed them and reported an + intact link as broken. + """ + text = heading.strip().lower() + text = re.sub(r"[^\w\s-]", "", text) + return text.replace(" ", "-") + + +def anchors(text): + return {anchor(m.group(1)) for m in (HEADING_RE.match(line) for line in text.splitlines()) if m} + + +def markdown_files(root): + for path in sorted(root.rglob("*.md")): + if SKIP_DIRS.isdisjoint(path.parts): + yield path + + +def check(root): + broken = [] + files = list(markdown_files(root)) + for path in files: + text = path.read_text(encoding="utf-8") + for match in LINK_RE.finditer(text): + target = match.group(2) + if target.startswith(EXTERNAL): + continue + file_part, _, fragment = target.partition("#") + if file_part: + referenced = (path.parent / file_part).resolve() + if not referenced.exists(): + broken.append((path, target, "no such file")) + continue + # A link into a non-Markdown file cannot be checked for anchors. + other = referenced.read_text(encoding="utf-8") if referenced.suffix == ".md" else None + else: + other = text + if fragment and other is not None and fragment not in anchors(other): + broken.append((path, target, "no such anchor")) + return files, broken + + +def main(): + root = pathlib.Path(sys.argv[1] if len(sys.argv) > 1 else ".").resolve() + files, broken = check(root) + print(f"{len(files)} file(s), {len(broken)} broken link(s)") + for path, target, reason in broken: + print(f" {path.relative_to(root)}: {target} -- {reason}") + return 1 if broken else 0 + + +if __name__ == "__main__": + sys.exit(main()) From 88cfc4df82b5d441c5b3e3a0d00e64ae0f3e3fd1 Mon Sep 17 00:00:00 2001 From: Patrick <320190286+muellerei@users.noreply.github.com> Date: Wed, 16 Sep 2026 19:56:35 +0200 Subject: [PATCH 2/3] Write down how the work is actually done, where the old wording did not hold Three practices earned their place on a single day of fixes, and the existing text would not have prevented any of them. A test has to be shown to fail before it is trusted. One meant to prove that search-pages matches on originalName searched for "Alpha", which .lower() also finds in name; it was green and tested nothing. Only removing the branch exposed it. Writes to a live graph leave pages behind. They use a zz-probe- prefix now and get deleted afterwards. "Update documentation" was too soft a word for a rule that was broken twice the day it applied: both flags in 0.10.0 went out without their README row and AGENTS.md entry. It is a checklist now, and it names --help, which is the source the tables are derived from. References name symbols, not line numbers -- a comment pointing at helpers.py:855 outlived its meaning within two commits. The link checker is mentioned where it is needed: at the end of the documentation checklist, since that is the change that moves anchors. --- CHANGELOG.md | 18 ++++++++++++++++++ CONTRIBUTING.md | 41 +++++++++++++++++++++++++++++++++++++---- 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 69a58bf..da895f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -86,6 +86,24 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 directions: an exemption for an option that no longer exists fails the suite rather than silently covering a future option that inherits the name. +- `CONTRIBUTING.md` says how work here is actually done, in the three places + where following the old wording would not have prevented the mistakes that + were made: a test has to be shown to fail before it is trusted, since one + meant to prove `search-pages` matches on `originalName` searched for a string + that `.lower()` also finds in `name` and so tested nothing; write tests + against a live graph use `zz-probe-` pages and delete them; and + "update documentation" is now a four-item checklist including `--help`, + because both flags in 0.10.0 went out without their README row and `AGENTS.md` + entry. References name symbols rather than line numbers — a comment pointing + at `helpers.py:855` outlived its meaning within two commits. + + `scripts/check-links.py` checks relative links and heading anchors across the + Markdown files. The anchor rule is the part that is easy to get wrong: GitHub + drops punctuation before turning spaces into hyphens, so an em dash in a + heading leaves both its spaces behind and the anchor takes two hyphens. It + skips generated and gitignored trees, after an earlier version read them and + reported a broken link no contributor could have seen. + ## [0.13.0] - 2026-09-16 ### Changed diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3a3f18b..a810e80 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -62,10 +62,40 @@ logseq-cli/ logseq-cli your-new-command ... ``` -4. **Update documentation.** If you add or change a command: - - Update `README.md` (command tables and usage examples) - - Update `AGENTS.md` (if it affects common workflows) - - Add an entry under `## [Unreleased]` in `CHANGELOG.md` + **Tests must be able to fail.** + + A test that confirms the fix instead of catching the bug is worth nothing + and looks like safety. Before trusting one, remove the fix and check that + the test goes red. + + This is not theory. A test meant to prove that `search-pages` also matches + on `originalName` searched for `"Alpha"` — which, after `.lower()`, is + present in `name` too. It passed, and it tested nothing; only removing the + `originalName` branch exposed it. The fixture now uses `Q&A / Support`, + whose ampersand does not survive into the slugged `name`. + + Tests that write to a live graph use throwaway pages with a recognisable + prefix — `zz-probe-` — and delete them afterwards. + +4. **A change is not done when the tests pass.** A new flag ships when it + appears in: + - `--help` — the option's own text, and the command epilog if the behaviour + is not obvious from the flag name + - the command table in `README.md` + - `AGENTS.md`, if it affects a common workflow + - `CHANGELOG.md` under `## [Unreleased]` + + Both flags added in 0.10.0 went out without the README row and the + `AGENTS.md` entry, and were caught the same evening. Twenty further options + had never been listed at all. `tests/test_readme_documents_options.py` now + holds the command table against the registry, which covers the README row + and nothing else on this list. + + Relative links and anchors across the Markdown files, after any of those: + + ```bash + python3 scripts/check-links.py . + ``` ## Design Principles @@ -93,6 +123,9 @@ logseq-cli/ so an unparseable `--from` exits 2 while a reversed range exits 1. That is a known inconsistency, not a pattern to copy. - **German + English.** `smart-query` keywords support both languages. +- **References name symbols, not line numbers.** A comment pointing at + `helpers.py:855` outlived its meaning within two commits; the function name + would not have. ## Reporting Issues From b1eed697c89b39fbe6eb4d6fbe2e566621134bce Mon Sep 17 00:00:00 2001 From: Patrick <320190286+muellerei@users.noreply.github.com> Date: Wed, 16 Sep 2026 20:03:37 +0200 Subject: [PATCH 3/3] Harden the link checker against the Markdown this repo actually contains Review found it wrong in both directions on input that is normal here. False positives: link syntax shown inside fenced blocks or inline code was read as a link, which matters because this project documents Markdown graphs and CONTRIBUTING itself shows link examples. Titled links, angle-bracketed targets, percent-encoded paths and links wrapped across lines were reported broken. Repeated headings -- the CHANGELOG has ten of "Fixed" -- resolve as #fixed-1 on GitHub, and that suffix was unknown here. False negative, the one that defeated the purpose: [text][ref] links were not matched at all, so a definition pointing at a deleted file passed silently. Both the reference and a label with no definition are checked now. An anchor is not only a heading: and id attributes make one too. Robustness: an unreadable file is reported and the run continues instead of ending in a traceback; a mistyped root exits 2 rather than reporting "0 files, 0 broken links" and passing. One trap found while fixing this, and it is why anchors are collected from the raw text: `### `[journal.headings]`` is a heading made entirely of inline code. Stripping code before reading headings deleted it, and a link that works on GitHub came back broken. CHANGELOG: the script is its own Added entry now. It is a new file, and the precedent here is examples/carried-over-todos.sh, which got one beside the behaviour change it shipped with rather than being folded into it. CONTRIBUTING: the search-pages example no longer says the fixture "now" uses Q&A -- the broken version never reached a commit, so git log would have contradicted it. What it teaches is the same; the mutation is what catches it. --- CHANGELOG.md | 42 ++++++---- CONTRIBUTING.md | 13 +-- scripts/check-links.py | 185 ++++++++++++++++++++++++++++++++++------- 3 files changed, 190 insertions(+), 50 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index da895f8..b624e65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- `scripts/check-links.py` checks the relative links and heading anchors across + the Markdown files. Every one of them claims a file and a heading exist, and + nothing verified that, so a rename broke them without any sign. The anchor + rule is the part that is easy to get wrong: GitHub drops punctuation before + turning spaces into hyphens, so an em dash in a heading leaves both its + spaces behind and the anchor takes two hyphens, not one. Link syntax shown + inside fenced blocks and inline code is not a link and is skipped — this + project documents Markdown graphs, so examples are the normal case. So are + generated trees and the gitignored `local/`, after an earlier version read + them and reported a break no contributor could have seen. + ### Fixed - `get-backlinks --with-context --limit` accepted a negative value and answered @@ -86,23 +99,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 directions: an exemption for an option that no longer exists fails the suite rather than silently covering a future option that inherits the name. -- `CONTRIBUTING.md` says how work here is actually done, in the three places - where following the old wording would not have prevented the mistakes that - were made: a test has to be shown to fail before it is trusted, since one - meant to prove `search-pages` matches on `originalName` searched for a string - that `.lower()` also finds in `name` and so tested nothing; write tests - against a live graph use `zz-probe-` pages and delete them; and - "update documentation" is now a four-item checklist including `--help`, - because both flags in 0.10.0 went out without their README row and `AGENTS.md` - entry. References name symbols rather than line numbers — a comment pointing - at `helpers.py:855` outlived its meaning within two commits. - - `scripts/check-links.py` checks relative links and heading anchors across the - Markdown files. The anchor rule is the part that is easy to get wrong: GitHub - drops punctuation before turning spaces into hyphens, so an em dash in a - heading leaves both its spaces behind and the anchor takes two hyphens. It - skips generated and gitignored trees, after an earlier version read them and - reported a broken link no contributor could have seen. +- `CONTRIBUTING.md` says how work here is actually done, in the places where + following the old wording would not have prevented the mistakes that were + made. A test has to be shown to fail before it is trusted: one written to + prove that `search-pages` matches on `originalName` would have passed while + testing nothing, because the obvious query string survives `.lower()` in + `name` as well — the fixture uses `Q&A / Support` instead, whose ampersand + does not survive being slugged. Tests that write to a live graph are to use + `zz-probe-` pages and delete them. And "update documentation" is a + four-item checklist now, `--help` included, because both flags in 0.10.0 went + out without their README row and `AGENTS.md` entry. References name symbols + rather than line numbers — a comment pointing at `helpers.py:855` outlived + its meaning within two commits. ## [0.13.0] - 2026-09-16 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a810e80..23658ea 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -68,11 +68,14 @@ logseq-cli/ and looks like safety. Before trusting one, remove the fix and check that the test goes red. - This is not theory. A test meant to prove that `search-pages` also matches - on `originalName` searched for `"Alpha"` — which, after `.lower()`, is - present in `name` too. It passed, and it tested nothing; only removing the - `originalName` branch exposed it. The fixture now uses `Q&A / Support`, - whose ampersand does not survive into the slugged `name`. + This is not theory. The test proving that `search-pages` also matches on + `originalName` first searched for `"Alpha"` — which, after `.lower()`, is + present in `name` too. It passed while testing nothing, and only removing + the `originalName` branch showed that: the query still matched. It searches + for `Q&A / Support` instead, whose ampersand does not survive into the + slugged `name`, and that one does go red when the branch is removed. The + fixture never reached a commit in its broken state, which is the point — + the mutation is what catches this, not review. Tests that write to a live graph use throwaway pages with a recognisable prefix — `zz-probe-` — and delete them afterwards. diff --git a/scripts/check-links.py b/scripts/check-links.py index 389b93a..27a10c5 100644 --- a/scripts/check-links.py +++ b/scripts/check-links.py @@ -11,24 +11,51 @@ External links (http, https, mailto) are not checked: that needs the network, and a 404 somewhere else is not the same class of error as a reference to our -own file that no longer resolves. +own file that no longer resolves. Images count as links -- a missing one is a +break the same way, and `![alt](x.png)` differs from a link only by a `!`. """ import pathlib import re import sys +import urllib.parse -# Generated or untracked trees hold Markdown too, and a checker that reads them -# reports failures nobody else can reproduce: `local/` is gitignored and exists -# only on one machine, `.pytest_cache/README.md` is written by pytest. Both were -# in scope in an earlier version, which is how a run over the repository came -# back with a broken link that no contributor could have seen. -SKIP_DIRS = {".git", "local", ".pytest_cache", ".venv", "venv", "node_modules"} +# Generated trees hold Markdown too, and reading them reports failures nobody +# else can reproduce: `.pytest_cache/README.md` is written by pytest. These are +# skipped wherever they appear, since a directory named `.git` is never ours at +# any depth. +SKIP_ANYWHERE = {".git", ".pytest_cache", ".venv", "venv", "node_modules"} + +# `local/` is different: it is gitignored and exists only on one machine, but +# the name is ordinary enough that a tracked `docs/local/` is plausible. Only +# the one at the root is skipped -- matching it at any depth would silently +# drop real documentation. +SKIP_AT_ROOT = {"local"} EXTERNAL = ("http://", "https://", "mailto:") -LINK_RE = re.compile(r"\[([^\]]+)\]\(([^)]+)\)") +# A link target may carry a title: [text](file.md "Title"). Splitting it off +# here keeps it out of the path; without that, the title was read as part of +# the filename and every titled link came back broken. +LINK_RE = re.compile(r"\[([^\]]*)\]\(\s*(<[^>]*>|[^\s)]+)(?:\s+\"[^\"]*\"|\s+'[^']*')?\s*\)") HEADING_RE = re.compile(r"^#{1,6}\s+(.*)$") +# [text][label] resolves through a definition line elsewhere in the file. The +# definition is what points at a file, so that is what gets checked; a label +# with no definition is a break of its own. +REF_LINK_RE = re.compile(r"\[[^\]]*\]\[([^\]]*)\]") +REF_DEF_RE = re.compile(r"^\s{0,3}\[([^\]]+)\]:\s*(<[^>]*>|\S+)", re.MULTILINE) + +# A heading is not the only thing that can be linked to: an or an id +# attribute makes an anchor too, and rejecting those would push contributors +# away from a technique the renderer supports. +HTML_ANCHOR_RE = re.compile(r"<[^>]*\b(?:name|id)\s*=\s*[\"']([^\"']+)[\"']") + +# Markdown that *shows* link syntax is not Markdown that *has* a link. This +# project documents Markdown graphs, so examples in fenced blocks and inline +# code are the normal case -- checking them turns documentation into a failure. +FENCE_RE = re.compile(r"^\s*(```|~~~)") +INLINE_CODE_RE = re.compile(r"`[^`]*`") + def anchor(heading): """Slug a heading the way GitHub does. @@ -45,45 +72,147 @@ def anchor(heading): def anchors(text): - return {anchor(m.group(1)) for m in (HEADING_RE.match(line) for line in text.splitlines()) if m} + """Every fragment this file can be linked to. + + Two headings that slug the same do not collide: GitHub appends `-1`, `-2` + to the later ones, so `#fixed-1` is a working link into a CHANGELOG with + repeated section headings. Counting them here is what makes that link + resolve instead of being reported as broken. + """ + found = set() + seen = {} + # Deliberately the raw text: a heading may consist entirely of inline code + # (`### `[journal.headings]``), and stripping code would delete the heading + # along with it, so a link that resolves on GitHub would read as broken. + for line in text.splitlines(): + heading = HEADING_RE.match(line) + if heading: + slug = anchor(heading.group(1)) + count = seen.get(slug, 0) + found.add(slug if count == 0 else f"{slug}-{count}") + seen[slug] = count + 1 + found.update(HTML_ANCHOR_RE.findall(line)) + return found def markdown_files(root): for path in sorted(root.rglob("*.md")): - if SKIP_DIRS.isdisjoint(path.parts): + parts = path.relative_to(root).parts + if SKIP_ANYWHERE.isdisjoint(parts) and parts[0] not in SKIP_AT_ROOT: yield path +def strip_code(text): + """Blank out fenced blocks and inline code, keeping line structure intact. + + Replacing rather than deleting keeps every other offset where it was, so + what is left still lines up with the file it came from. + """ + out = [] + fence = None + for line in text.splitlines(): + marker = FENCE_RE.match(line) + if fence: + out.append("") + if marker and marker.group(1) == fence: + fence = None + continue + if marker: + fence = marker.group(1) + out.append("") + continue + out.append(INLINE_CODE_RE.sub("", line)) + return "\n".join(out) + + +def read(path): + """Read a file, or report why it could not be read instead of crashing. + + A checker that dies on one unreadable file tells a contributor less than a + stack trace's worth of nothing: the run stops before reaching the files + that were fine. + """ + try: + return path.read_text(encoding="utf-8"), None + except (UnicodeDecodeError, OSError) as exc: + return None, type(exc).__name__ + + +def check_target(path, text, target): + """Resolve one link target; return a reason if it does not hold, else None.""" + if target.startswith("<") and target.endswith(">"): + target = target[1:-1] + if target.startswith(EXTERNAL) or "://" in target: + return None + file_part, _, fragment = target.partition("#") + # A path may be percent-encoded in the link and plain on disk. + file_part = urllib.parse.unquote(file_part) + fragment = urllib.parse.unquote(fragment) + if file_part: + referenced = (path.parent / file_part).resolve() + if not referenced.exists(): + return "no such file" + if referenced.suffix != ".md": + # Anchors cannot be checked in a non-Markdown file. + return None + other, error = read(referenced) + if other is None: + return f"unreadable target ({error})" + else: + other = text + if fragment and fragment not in anchors(other): + return "no such anchor" + return None + + +def check_file(path, text): + broken = [] + body = strip_code(text) + definitions = {m.group(1).lower(): m.group(2) for m in REF_DEF_RE.finditer(body)} + for match in LINK_RE.finditer(body): + reason = check_target(path, text, match.group(2)) + if reason: + broken.append((path, match.group(2), reason)) + for match in REF_LINK_RE.finditer(body): + label = match.group(1).lower() + if not label: + continue + if label not in definitions: + broken.append((path, f"[{match.group(1)}]", "no such link definition")) + continue + reason = check_target(path, text, definitions[label]) + if reason: + broken.append((path, definitions[label], reason)) + return broken + + def check(root): broken = [] files = list(markdown_files(root)) for path in files: - text = path.read_text(encoding="utf-8") - for match in LINK_RE.finditer(text): - target = match.group(2) - if target.startswith(EXTERNAL): - continue - file_part, _, fragment = target.partition("#") - if file_part: - referenced = (path.parent / file_part).resolve() - if not referenced.exists(): - broken.append((path, target, "no such file")) - continue - # A link into a non-Markdown file cannot be checked for anchors. - other = referenced.read_text(encoding="utf-8") if referenced.suffix == ".md" else None - else: - other = text - if fragment and other is not None and fragment not in anchors(other): - broken.append((path, target, "no such anchor")) + text, error = read(path) + if text is None: + broken.append((path, "", f"unreadable ({error})")) + continue + broken.extend(check_file(path, text)) return files, broken def main(): + if len(sys.argv) > 2: + print("usage: check-links.py [ROOT]", file=sys.stderr) + return 2 root = pathlib.Path(sys.argv[1] if len(sys.argv) > 1 else ".").resolve() + # Without this a mistyped path reports "0 files, 0 broken links" and exits + # green -- a check that silently verified nothing. + if not root.is_dir(): + print(f"not a directory: {root}", file=sys.stderr) + return 2 files, broken = check(root) print(f"{len(files)} file(s), {len(broken)} broken link(s)") for path, target, reason in broken: - print(f" {path.relative_to(root)}: {target} -- {reason}") + where = path.relative_to(root) + print(f" {where}: {target} -- {reason}" if target else f" {where}: {reason}") return 1 if broken else 0