Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs(patterns): Comments for fixed-length heterogeneous collection patterns #2291

Merged
merged 1 commit into from
May 19, 2024

Conversation

kriskowal
Copy link
Member

@kriskowal kriskowal commented May 16, 2024

Until we have a rigorous treatment on the usage of patterns, this adds a bandaid to the code itself so anyone looking for how to make a pattern for a struct-like CopyRecord, tuple-like CopyArray, or fixed-length heterogeneous CopyMap will find their way.

Copy link
Contributor

@erights erights left a comment

Choose a reason for hiding this comment

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

Looks good so far. Do this symmetrically for CopyArrays, Taggeds, CopyMaps, and passable primitives. I think that covers all the cases, but you should double check whether I forgot anything.

@kriskowal kriskowal requested a review from erights May 16, 2024 01:42
@kriskowal kriskowal force-pushed the kriskowal-patterns-struct branch 4 times, most recently from 7ff741e to 72e6090 Compare May 16, 2024 01:48
@@ -1651,13 +1651,26 @@ const makePatternKit = () => {
limits ? makeLimitsMatcher('match:symbol', [limits]) : SymbolShape,
record: (limits = undefined) =>
limits ? M.recordOf(M.any(), M.any(), limits) : RecordShape,
// The pattern for a CopyRecord with a fixed quantity of heterogeneous
// entry types (a Struct) is merely a hardened object with the respective
Copy link
Contributor

Choose a reason for hiding this comment

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

x

Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be

Suggested change
// entry types (a Struct) is merely a hardened object with the respective
// value types (a Struct) is merely a hardened object with the respective

Also, I'm uncomfortable making the connection to types in a way that sound stronger than just a metaphor. TS types are both more and less expressive than patterns. Neither is a subset of the other.

Suggested change
// entry types (a Struct) is merely a hardened object with the respective
// value patterns (a Struct) is merely a hardened object with the respective

That "hardened object" is itself a CopyRecord btw. Put more simply, a CopyRecord whose property values are patterns is itself a pattern. I don't know if this helps.

Copy link
Member Author

Choose a reason for hiding this comment

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

I rewrote all sections to convey this clarification.

// The pattern for a CopyMap with a fixed quantity of heterogeneous entry
// types is merely a CopyMap of corresponding key and value patterns, for
// example,
// makeCopyMap([[M.string(), M.number()], [M.number(), M.string()]]).
Copy link
Contributor

Choose a reason for hiding this comment

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

No. The keys must be keys. Only the values can be non-key patterns.

Copy link
Member Author

Choose a reason for hiding this comment

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

Rewrote record and map to clarify this distinction.

@kriskowal kriskowal requested a review from erights May 16, 2024 02:35
@kriskowal kriskowal changed the title docs(patterns): Bandaid on struct-like record pattern documentation docs(patterns): Bandaid on fixed-length heterogeneous collection patterns May 16, 2024
@kriskowal kriskowal changed the title docs(patterns): Bandaid on fixed-length heterogeneous collection patterns docs(patterns): Comments for fixed-length heterogeneous collection patterns May 16, 2024
@kriskowal kriskowal force-pushed the kriskowal-patterns-struct branch 3 times, most recently from 6ce1307 to 705a32c Compare May 16, 2024 04:25
@kriskowal
Copy link
Member Author

Looks good so far. Do this symmetrically for CopyArrays, Taggeds, CopyMaps, and passable primitives. I think that covers all the cases, but you should double check whether I forgot anything.

Rewrote all sections to speak in terms of the copy data that each pattern matches instead of using TypeScript. Covered CopyRecord, CopyArray, and CopyMaps. Tags aren’t heterogeneous collections. CopySet and CopyBag are collections but can’t be fixed-length and heterogeneous with types for corresponding keys or indexes, so they’re out.

Copy link
Contributor

@erights erights left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@kriskowal kriskowal enabled auto-merge May 19, 2024 01:26
@kriskowal kriskowal merged commit 2f3b66b into master May 19, 2024
17 checks passed
@kriskowal kriskowal deleted the kriskowal-patterns-struct branch May 19, 2024 01:34
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.

None yet

2 participants