breaking-change: pattern is now the primary constructor#40
Conversation
…ns with value + elements; `point` is the special constructor for value-only construction
There was a problem hiding this comment.
Pull request overview
This pull request implements a breaking change to the Pattern library's constructor API, making pattern the primary constructor for patterns with elements (taking a value and a list of elements), while point (and pure) become the standard constructors for atomic patterns. The patternWith function is being removed from the public API.
Key Changes:
patternnow takes two arguments (value and elements list) and is the primary constructorpointis now the recommended constructor for atomic patterns (patterns with no elements)patternWithhas been removed from the public API exports- All documentation, examples, and test files updated to reflect the new convention
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 28 comments.
Show a summary per file
| File | Description |
|---|---|
| specs/013-applicative-instance/quickstart.md | Updated all examples to use pattern for patterns with elements and point for atomic patterns |
| specs/004-construction-functions/quickstart.md | Revised tutorial documentation to reflect new API with point for atomic patterns and pattern for patterns with elements |
| specs/004-construction-functions/data-model.md | Updated data model documentation to describe the new constructor roles |
| specs/004-construction-functions/contracts/type-signatures.md | Updated API signatures and documentation, replacing patternWith references with pattern |
| libs/pattern/tests/Spec/Pattern/Properties.hs | Updated property tests to use new constructor names and updated import statements |
| libs/pattern/tests/Spec/Pattern/GraphSpec.hs | Updated graph tests to use pattern and point consistently |
| libs/pattern/src/Pattern/Core.hs | Critical issues found: Removed patternWith from exports and renamed constructor functions, but many doctest examples still use old pattern syntax for atomic patterns |
| libs/pattern/examples/examples.md | Updated usage examples throughout to demonstrate new constructor API |
| docs/users/api/pattern-construction.md | Revised API documentation to reflect pattern's new role and clarify when to use each constructor |
| README.md | Updated roadmap to reflect new constructor terminology |
Critical Issues Found:
The Pattern.Core.hs file has extensive inconsistencies where doctest examples throughout the file still use pattern with a single argument (the old atomic pattern syntax), but pattern now requires two arguments. These need to be changed to either point or the full form pattern value []. There are approximately 28 instances of this issue across multiple functions' documentation, affecting:
- Typeclass instance examples (Monoid, Hashable, Functor, Traversable)
- Query function examples (length, size, depth, values, anyValue, allValues)
- Pattern predicate function examples (filterPatterns, findPattern, findAllPatterns, contains)
- Context function examples (toTuple, depthAt, sizeAt, indicesAt)
- Variable reference errors where examples use wrong variable names
These doctest failures will cause compilation or testing issues and need to be corrected for the breaking change to be complete.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| -- >>> elem1 = pattern 5 | ||
| -- >>> elem2 = pattern 10 |
There was a problem hiding this comment.
Inconsistent usage of pattern function. Lines 821-822 use elem1 = pattern 5 and elem2 = pattern 10 which now require two arguments (value and elements list). These should be elem1 = point 5 and elem2 = point 10 for atomic patterns.
| -- Nested pattern structure: | ||
| -- | ||
| -- >>> let validate x = if x > 0 then Just x else Nothing | ||
| -- >>> inner = pattern 1 |
There was a problem hiding this comment.
Inconsistent usage of pattern function. Line 830 uses inner = pattern 1 which now requires two arguments (value and elements list). This should be inner = point 1 for an atomic pattern.
| -- >>> inner = pattern 1 | |
| -- >>> inner = point 1 |
| @@ -928,10 +915,10 @@ length (Pattern _ es) = Prelude.length es | |||
| -- >>> size (pattern "atom") | |||
There was a problem hiding this comment.
Inconsistent usage of pattern function. Line 915 uses size (pattern "atom") which now requires two arguments (value and elements list). This should be size (point "atom") for an atomic pattern, or size (pattern "atom" []) if using the new form.
| @@ -967,7 +954,7 @@ depth (Pattern _ es) = 1 + maximum (map depth es) | |||
| -- >>> values (pattern "atom") | |||
There was a problem hiding this comment.
Inconsistent usage of pattern function. Line 954 uses values (pattern "atom") which now requires two arguments (value and elements list). This should be values (point "atom") for an atomic pattern, or values (pattern "atom" []) if using the new form.
| -- >>> values (pattern "atom") | |
| -- >>> values (point "atom") |
| -- ["atom"] | ||
| -- | ||
| -- >>> values (patternWith "root" [pattern "a", pattern "b"]) | ||
| -- >>> values (pattern "root" [pattern "a", pattern "b"]) |
There was a problem hiding this comment.
Inconsistent usage of pattern function. Line 957 uses pattern "a" and pattern "b" which now require two arguments (value and elements list). These should be point "a" and point "b" for atomic patterns.
| -- Just (Pattern 1 [Pattern 2 []]) | ||
| -- | ||
| -- >>> invalid = patternWith 1 [pattern (-1)] | ||
| -- >>> invalid = pattern 1 [pattern (-1)] |
There was a problem hiding this comment.
Inconsistent usage of pattern function. Line 802 uses pattern (-1) which now requires two arguments (value and elements list). This should be point (-1) for an atomic pattern.
| -- 2 | ||
| -- | ||
| -- >>> size (patternWith "root" [pattern "a", pattern "b"]) | ||
| -- >>> size (pattern "root" [pattern "a", pattern "b"]) |
There was a problem hiding this comment.
Inconsistent usage of pattern function. Line 921 uses pattern "a" and pattern "b" which now require two arguments (value and elements list). These should be point "a" and point "b" for atomic patterns.
| -- >>> size (pattern "root" [pattern "a", pattern "b"]) | |
| -- >>> size (pattern "root" [point "a", point "b"]) |
| -- === Examples | ||
| -- | ||
| -- >>> p = patternWith 3 [pattern 6, pattern 2] | ||
| -- >>> p = pattern 3 [pattern 6, pattern 2] |
There was a problem hiding this comment.
Inconsistent usage of pattern function. Line 1004 uses pattern 3, pattern 6, and pattern 2 which now require two arguments (value and elements list). These should be point 3, point 6, and point 2 for atomic patterns.
| -- === Examples | ||
| -- | ||
| -- >>> p = patternWith 3 [pattern 6, pattern 2] | ||
| -- >>> p = pattern 3 [pattern 6, pattern 2] |
There was a problem hiding this comment.
Inconsistent usage of pattern function. Line 1022 uses pattern 3, pattern 6, and pattern 2 which now require two arguments (value and elements list). These should be point 3, point 6, and point 2 for atomic patterns.
| -- === Examples | ||
| -- | ||
| -- >>> p = patternWith "root" [pattern "child"] | ||
| -- >>> p = pattern "root" [pattern "child"] |
There was a problem hiding this comment.
Inconsistent usage of pattern function. Line 1120 uses pattern "child" which now requires two arguments (value and elements list). This should be point "child" for an atomic pattern.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 11 changed files in this pull request and generated 19 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| -- ["atom"] | ||
| -- | ||
| -- >>> values (patternWith "root" [pattern "a", pattern "b"]) | ||
| -- >>> values (pattern "root" [pattern "a", pattern "b"]) |
There was a problem hiding this comment.
This doctest example uses pattern "a" and pattern "b" to create atomic patterns on line 957, but according to the new convention, they should use point "a" and point "b" instead. The pattern function is now the primary constructor for patterns with elements, while point is used for atomic patterns.
| -- >>> p = pattern "root" [pattern "child"] | ||
| -- >>> contains p (pattern "child") |
There was a problem hiding this comment.
These doctest examples use pattern "child" and pattern "missing" to create atomic patterns on lines 1074 and 1077, but according to the new convention, they should use point "child" and point "missing" instead. The pattern function is now the primary constructor for patterns with elements, while point is used for atomic patterns.
| The Pattern library provides three functionally equivalent ways to create atomic patterns (patterns with no elements) from a value: | ||
|
|
||
| 1. **`point`** - Category-theory aligned name (pointed functor) | ||
| 2. **`pattern`** - Domain-specific name (creates a Pattern) | ||
| 3. **`pure`** - Applicative typeclass method | ||
| 2. **`pure`** - Applicative typeclass method | ||
|
|
||
| All three functions are identical in behavior: they take a value and return an atomic pattern containing that value with an empty elements list. |
There was a problem hiding this comment.
The documentation states "three functionally equivalent ways" but only lists two: point and pure. Since pattern is no longer used for atomic patterns, this should be updated to say "two functionally equivalent ways".
| -- Just (Pattern 1 [Pattern 2 []]) | ||
| -- | ||
| -- >>> invalid = patternWith 1 [pattern (-1)] | ||
| -- >>> invalid = pattern 1 [pattern (-1)] |
There was a problem hiding this comment.
This doctest example on line 802 uses pattern (-1) to create an atomic pattern, but according to the new convention, it should use point (-1) instead. The pattern function is now the primary constructor for patterns with elements, while point is used for atomic patterns.
| -- >>> invalid = pattern 1 [pattern (-1)] | |
| -- >>> invalid = pattern 1 [point (-1)] |
| @@ -913,7 +900,7 @@ fromList v vs = patternWith v (map pattern vs) | |||
| -- >>> length (pattern "atom") | |||
There was a problem hiding this comment.
This doctest example uses pattern "atom" with only one argument, which would create a pattern with value "atom" and an empty list as elements. However, this is treating pattern as if it were a curried function waiting for the second argument. The example should use point "atom" to create an atomic pattern, or pattern "atom" [] to be explicit about the empty elements list.
| -- >>> length (pattern "atom") | |
| -- >>> length (point "atom") |
| -- >>> anyValue (> 5) (pattern 3 [pattern 6, pattern 2]) | ||
| -- True | ||
| -- | ||
| -- >>> anyValue (> 10) (patternWith 3 [pattern 6, pattern 2]) | ||
| -- >>> anyValue (> 10) (pattern 3 [pattern 6, pattern 2]) |
There was a problem hiding this comment.
These doctest examples use pattern 6 and pattern 2 to create atomic patterns on lines 972 and 975, but according to the new convention, they should use point 6 and point 2 instead. The pattern function is now the primary constructor for patterns with elements, while point is used for atomic patterns.
| -- >>> let validate x = if x > 0 then Just x else Nothing | ||
| -- >>> pattern = patternWith 1 [pattern 2] | ||
| -- >>> p = pattern 1 [point 2] | ||
| -- >>> traverse validate pattern |
There was a problem hiding this comment.
This doctest example references the variable pattern on line 799 but the code creates a variable named p on line 798. This should be traverse validate p.
| -- Sequencing effects: | ||
| -- | ||
| -- >>> pattern = patternWith (Just 1) [pattern (Just 2)] | ||
| -- >>> pattern = pattern (Just 1) [pattern (Just 2)] |
There was a problem hiding this comment.
This doctest example on line 808 uses pattern (Just 1) and pattern (Just 2) to create patterns, but pattern (Just 2) should be point (Just 2) since it's creating an atomic pattern. The pattern function is now the primary constructor for patterns with elements, while point is used for atomic patterns.
| -- >>> pattern = pattern (Just 1) [pattern (Just 2)] | |
| -- >>> pattern = pattern (Just 1) [point (Just 2)] |
| -- Nested pattern structure: | ||
| -- | ||
| -- >>> let validate x = if x > 0 then Just x else Nothing | ||
| -- >>> inner = pattern 1 |
There was a problem hiding this comment.
This doctest example uses pattern 1 on line 830 to create an atomic pattern, but according to the new convention, it should use point 1 instead. The pattern function is now the primary constructor for patterns with elements, while point is used for atomic patterns.
| -- >>> inner = pattern 1 | |
| -- >>> inner = point 1 |
| -- === Examples | ||
| -- | ||
| -- >>> p = patternWith "root" [pattern "a", pattern "b"] | ||
| -- >>> p = pattern "root" [pattern "a", pattern "b"] |
There was a problem hiding this comment.
This doctest example uses pattern "a" and pattern "b" to create atomic patterns on line 1133, but according to the new convention, they should use point "a" and point "b" instead. The pattern function is now the primary constructor for patterns with elements, while point is used for atomic patterns.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 12 changed files in this pull request and generated 16 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| -- Sequencing effects: | ||
| -- | ||
| -- >>> pattern = patternWith (Just 1) [pattern (Just 2)] | ||
| -- >>> pattern = pattern (Just 1) [point (Just 2)] |
There was a problem hiding this comment.
Variable name pattern conflicts with the function name pattern. In Haskell, you cannot use the same name for both a variable and a function in scope. This should be changed to a different variable name such as p or pat.
| -- >>> pattern = pattern (Just 1) [point (Just 2)] | |
| -- >>> pat = pattern (Just 1) [point (Just 2)] |
| -- >>> pattern = patternWith "greeting" [elem1, elem2] | ||
| -- >>> elem1 = point "hello" | ||
| -- >>> elem2 = point "world" | ||
| -- >>> pattern = pattern "greeting" [elem1, elem2] |
There was a problem hiding this comment.
Variable name pattern conflicts with the function name pattern. In Haskell, you cannot use the same name for both a variable and a function in scope. This should be changed to a different variable name such as p or pat.
| -- >>> pattern = patternWith 20 [elem1, elem2] | ||
| -- >>> elem1 = point 5 | ||
| -- >>> elem2 = point 10 | ||
| -- >>> pattern = pattern 20 [elem1, elem2] |
There was a problem hiding this comment.
Variable name pattern conflicts with the function name pattern. In Haskell, you cannot use the same name for both a variable and a function in scope. This should be changed to a different variable name such as p or pat.
| -- >>> inner = point "inner" | ||
| -- >>> middle = pattern "middle" [inner] | ||
| -- >>> outer = pattern "outer" [middle] | ||
| -- >>> pattern = pattern "root" [outer] |
There was a problem hiding this comment.
Variable name pattern conflicts with the function name pattern. In Haskell, you cannot use the same name for both a variable and a function in scope. This should be changed to a different variable name such as p or pat.
| -- >>> pattern = patternWith "20" [elem1, elem2] | ||
| -- >>> elem1 = point "5" | ||
| -- >>> elem2 = point "10" | ||
| -- >>> pattern = pattern "20" [elem1, elem2] |
There was a problem hiding this comment.
Variable name pattern conflicts with the function name pattern. In Haskell, you cannot use the same name for both a variable and a function in scope. This should be changed to a different variable name such as p or pat.
| -- >>> branch1 = pattern "b1" [point "b1leaf"] | ||
| -- >>> branch2 = pattern "b2" [pattern "b2mid" [point "b2leaf"]] | ||
| -- >>> branch3 = point "b3" | ||
| -- >>> pattern = pattern "root" [branch1, branch2, branch3] |
There was a problem hiding this comment.
Variable name pattern conflicts with the function name pattern. In Haskell, you cannot use the same name for both a variable and a function in scope. This should be changed to a different variable name such as p or pat.
| -- >>> pattern = patternWith 20 [elem1, elem2] | ||
| -- >>> elem1 = point 5 | ||
| -- >>> elem2 = point 10 | ||
| -- >>> pattern = pattern 20 [elem1, elem2] |
There was a problem hiding this comment.
Variable name pattern conflicts with the function name pattern. In Haskell, you cannot use the same name for both a variable and a function in scope. This should be changed to a different variable name such as p or pat.
| -- >>> elem1 = point 5 | ||
| -- >>> elem2 = point 10 | ||
| -- >>> pattern = pattern 20 [elem1, elem2] | ||
| -- >>> traverse validate pattern |
There was a problem hiding this comment.
Variable name pattern conflicts with the function name pattern. In Haskell, you cannot use the same name for both a variable and a function in scope. This should be changed to a different variable name such as p or pat.
| -- >>> pattern = patternWith 3 [middle] | ||
| -- >>> inner = point 1 | ||
| -- >>> middle = pattern 2 [inner] | ||
| -- >>> pattern = pattern 3 [middle] |
There was a problem hiding this comment.
Variable name pattern conflicts with the function name pattern. In Haskell, you cannot use the same name for both a variable and a function in scope. This should be changed to a different variable name such as p or pat.
| -- >>> level3 = pattern "level3" [level4] | ||
| -- >>> level2 = pattern "level2" [level3] | ||
| -- >>> level1 = pattern "level1" [level2] | ||
| -- >>> pattern = pattern "root" [level1] |
There was a problem hiding this comment.
Variable name pattern conflicts with the function name pattern. In Haskell, you cannot use the same name for both a variable and a function in scope. This should be changed to a different variable name such as p or pat.
|
@copilot open a new pull request to apply changes based on the comments in this thread |
|
@akollegger I've opened a new pull request, #41, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: akollegger <53756+akollegger@users.noreply.github.com>
Fix variable naming conflicts in doctests and documentation accuracy
patternis now the primary constructor for patterns with value + elements;pointis the special constructor for value-only constructionThis pull request updates the Pattern library's documentation and examples to clarify and standardize the usage of pattern construction functions. The main change is the deprecation of the
patternWithfunction in favor of usingpatternfor constructing patterns with elements, whilepoint(andpure) are now the recommended ways to create atomic patterns. All documentation, examples, and API references have been updated to reflect this new convention.Pattern construction API and documentation updates:
point(andpure) are used for atomic patterns, whilepatternis the primary constructor for patterns with elements. Removed references topatternWiththroughout the docs and examples, replacing them with the new usage. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12]Code and API surface changes:
patternWithfunction from the public API inPattern.Core, so users now usepatternfor patterns with elements andpointfor atomic patterns.Pattern.Coreto usepointfor atomic patterns andpatternfor patterns with elements, ensuring consistency across the codebase. [1] [2] [3] [4] [5] [6] [7] [8] [9]Project documentation updates:
README.mdto reflect the new terminology and function roles for pattern construction.These changes improve the clarity and usability of the Pattern library by making the construction of atomic and composite patterns more explicit and consistent.