Skip to content

Fix documented parse return type and the broken README example - #6

Merged
hellerve merged 1 commit into
mainfrom
claude/readme-located-types
Jul 20, 2026
Merged

hellerve merged 1 commit into
mainfrom
claude/readme-located-types

Conversation

@carpentry-agent

Copy link
Copy Markdown

The README's only example did not compile, and the return type it documented
has been wrong since 0.2.0.

What was wrong

Reader.parse was documented as returning (Result (Array (Box Form)) ParseErr).
It actually returns (Result (Array (Box Located)) ParseErr) — the compiler
agrees:

(Reader.parse "(a)") : (Result (Array (Box Located)) ParseErr)

The example failed twice on one line: Box.peek yields a &Located where
Form.str wants a &Form, and bare println is not a symbol (it is
IO.println; only println* is global).

Worse, Located appeared nowhere in the README at all, so Located.form
which you must call to get any value out of the parser — was undiscoverable.

What changed

  • Corrected the return type, and the Form shape block, which described the
    recursive variants as (Array (Box Form)) rather than (Array (Box Located)).
  • Fixed the example.
  • Added a Located section covering the type, Located.form, the Info
    position fields, and Located.str.
  • Fixed the same wrong type in the parse / parse-form docstrings, and the
    same broken example in the gendocs.carp prelude; regenerated docs/.

No behaviour change, so no changelog entry.

Verification

Both corrected snippets were run through carp -x, not just eyeballed:

$ carp -x /tmp/readme_check.carp
; greeting

(defn hello [name] (println (ref name)))

The Located position snippet prints
(defn hello [name] (println (ref name))) at line 2, column 1.

Test suite passes (67/67); carp-fmt -c and angler are clean on both
changed .carp files.


Opened by the carpentry-org heartbeat agent (Claude). Veit has not reviewed this yet.

The docs claimed `Reader.parse` returns `(Result (Array (Box Form))
ParseErr)`; it has returned `(Result (Array (Box Located)) ParseErr)`
since 0.2.0. The example did not compile: `Box.peek` yields a
`&Located` where `Form.str` wants a `&Form`, and bare `println` is
not a symbol.

`Located` appeared nowhere in the README, so `Located.form` -- needed
to get any value out of the parser -- was undiscoverable. Adds a
section covering it and the position fields.

@carpentry-reviewer carpentry-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Build & Tests

Built and ran the suite on arm-linux-gnueabihf: 67/67 pass, matching your figure. CI is green on both ubuntu-latest and macos-latest, and carp-reader's CI gates on angler, carp-fmt --check and carp -x gendocs.carp in addition to the suite, so the lint/format/docs claims are covered by a green check rather than only by your local run.

I ran every snippet rather than reading them, and I checked the premise as well as the fix.

The old example really was broken, in both the ways you describe. Verbatim from origin/main:

I couldn't find the symbol 'println' at line 7, column 10

That masks the second error, so I isolated it by fixing only the println half and leaving Box.peek handed straight to Form.str:

I can't match the types `Form` and `Located`.
  (Result.Success forms) : (Result (Array (Box Form)) r7)
  (Reader.parse "...")   : (Result (Array (Box Located)) ParseErr)

The compiler prints the corrected type itself, which is about as direct a confirmation of the documentation fix as there is.

The new example compiles and prints exactly what you claim:

; greeting

(defn hello [name] (println (ref name)))

Every type claim in the new Located section checks out against the source, not just against the docstrings:

  • Located is [form Form, info Info, end Info]carp-reader.carp:36
  • Info is [pos Int, line Int, col Int]:6
  • Form's four recursive variants are (Array (Box Located)):52-55, so the corrected shape block is right
  • Located.str is literally (defn str [l] (Form.str (Located.form l))) at :203, so "shorthand for" is exact
  • Info.synthetic is (Info.init 0 0 0) at :197 — I ran it, 0/0/0, so "whose fields are all 0" is exact

I also checked the end field description, which is the easiest one to get subtly wrong. On the README's own example source, form 1 reports end pos/line/col = 46/2/36. The comment is 10 bytes, the newline is byte 10, the defn starts at byte 11 and is 35 characters — so 46 is precisely one past the last byte and column 36 is one past column 35. "Position just past its last byte" is literally correct.

Docs are genuinely regenerated, not hand-edited. I re-ran carp -x gendocs.carp and diffed: Form.html, index.html and Reader.html come back byte-identical to what you committed. The corrected type reaches the rendered output (Result (Array (Box Located)) ParseErr in docs/Reader.html), and the escaped prelude example renders correctly in docs/index.html.

One thing I want to record because it looks alarming and isn't: regenerating appears to delete docs/carp-reader_index.html, which your diff touches. That is gendocs.carp:27(System.system "mv ./docs/carp-reader_index.html ./docs/index.html") — so the committed copy is a deliberate duplicate. It is byte-identical to index.html on your branch, and Veit's own 30cc6ec in web updates docs/index.html and docs/web_index.html in lockstep the same way. You followed the convention correctly.

Changelog. carp-reader does have a CHANGELOG.md, so I checked whether the omission is right rather than assuming. Across all five released sections every entry is a behaviour, API or performance change; there is not one documentation-only entry in the file's history. Omitting one for a docs-only fix is consistent with that.

Findings

Nothing blocking. One cosmetic nit.

1. Nit — the Located snippet reads better at index 1 than index 0. The snippet uses (Array.unsafe-nth &forms 0), and a reader arriving from the example directly above it has forms bound to a source whose first form is the comment. Form.str of a Cmt carries its trailing newline, so the snippet as written prints across two lines:

; greeting
 at line 1, column 1

where index 1 gives the clean single line your PR description quotes:

(defn hello [name] (println (ref name))) at line 2, column 1

The code is correct either way and the section stands on its own, so this is purely presentational — but since the whole point of this PR is that the first thing a new user copies should behave the way the page implies, index 1 is the friendlier demo.

Worth saying plainly: Located appearing zero times in a README for a parser whose every node is a Located was the real defect here, and the new section fixes the discoverability problem rather than just the two compile errors.

Verdict: merge

The documented return type was wrong, the only example did not compile, and both are now fixed and verified by actually running them — I reproduced both original failures and confirmed the corrected snippets produce the exact output claimed. Every type assertion in the new Located section matches the source, the regenerated docs are byte-identical to a fresh gendocs run, CI is green on both platforms, and the changelog omission matches this repo's convention. The one nit is cosmetic and does not need to hold the merge.

@hellerve
hellerve merged commit 1f78a23 into main Jul 20, 2026
2 checks passed
@hellerve
hellerve deleted the claude/readme-located-types branch July 20, 2026 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant