Skip to content

Conversation

@adamchalmers
Copy link
Contributor

@adamchalmers adamchalmers commented Nov 4, 2025

The "clean up type string" logic here is a bit complicated, and it doesn't support arrays of unions. IMO it's difficult to do both formatting and parsing of KCL types in the same function. A better approach would be to recursively parse the type into a tree structure (where arrays have a subtype, and unions have multiple subtypes), then format that tree.

This PR:

  • Adds unit tests for the cleanup_type_string function, in the first commit.
  • Adds a new proper recursive parser for KCL type strings (parses them into a tree)
  • Changes the KCL type formatter cleanup_type_string to use this parsed tree, rather than adhoc string processing. This includes changes to the unit tests added in the first commit.
  • Updates all the docs

This has some very slight changes to the resulting Markdown, which should be easy to see. They're basically change how arrays are nested in their markdown links.

As a side-effect this should unblock Serena's PR which adds array of unions.

Old: the Markdown link contains a [string] in its text
New: the Markdown link is just around string, and the surrounding array braces are outside the link.

@adamchalmers adamchalmers requested a review from a team as a code owner November 4, 2025 00:46
@vercel
Copy link

vercel bot commented Nov 4, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
modeling-app Ready Ready Preview Comment Nov 4, 2025 1:58am

@adamchalmers adamchalmers changed the title KCL: Add unit tests for cleanup_type_string KCL: New KCL type formatter and parser Nov 4, 2025
@adamchalmers adamchalmers requested a review from a team as a code owner November 4, 2025 01:24
The "clean up type string" logic [here](https://github.com/KittyCAD/modeling-app/blob/achalmers/array-of-unions/rust/kcl-lib/src/docs/gen_std_tests.rs#L517) is a bit complicated, and it doesn't support arrays of unions. IMO it's difficult to do both formatting and parsing of KCL types in the same function. A better approach would be to recursively parse the type into a tree structure (where arrays have a subtype, and unions have multiple subtypes), then format that tree.

This PR just parses the type tree, and runs it through all the type names we generate while building KCL's docs. A follow-up PR will then traverse that tree to build the nicely-formatted type.
@adamchalmers adamchalmers merged commit c1c5323 into main Nov 4, 2025
75 of 76 checks passed
@adamchalmers adamchalmers deleted the achalmers/unit-tests branch November 4, 2025 20:28
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.

3 participants