-
Notifications
You must be signed in to change notification settings - Fork 42
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
Fix combineSpans #61
Comments
Hey @ChrisPenner I'm giving this a shot. Can I check with you that the tests below reflect the semantics we're after? describe "combineSpans" $ do
-- semantics are that in `Range start end`
-- - start is inclusive.
-- - end is exclusive.
it "should ignore spans with 0 size ranges" $ do
combineSpans
[ Span (Range (Coord 0 0) (Coord 0 0)) "a"
, Span (Range (Coord 0 0) (Coord 0 0)) "b"
]
`shouldBe`
[
]
-- expected: []
-- but got: [((Coord 0 0),"a"),((Coord 0 0),""),((Coord 0 0),"b"),((Coord 0 0),"")]
it "should combine spans containing mempty" $ do
combineSpans
[ Span (Range (Coord 0 0) (Coord 0 2)) ""
, Span (Range (Coord 0 0) (Coord 0 2)) ""
]
`shouldBe`
[ (Coord 0 0, "")
]
-- expected: [((Coord 0 0),"")]
-- but got: [((Coord 0 0),""),((Coord 0 0),""),((Coord 0 2),""),((Coord 0 2),"")]
it "should combine consecutive spans" $ do
combineSpans
[ Span (Range (Coord 0 0) (Coord 0 1)) "a"
, Span (Range (Coord 0 1) (Coord 0 2)) "b"
]
`shouldBe`
[ (Coord 0 0,"a")
, (Coord 0 1,"b")
, (Coord 0 2,"")
]
-- expected: [((Coord 0 0),"a"),((Coord 0 1),"b"),((Coord 0 2),"")]
-- but got: [((Coord 0 0),"a"),((Coord 0 1),""),((Coord 0 1),"b"),((Coord 0 2),"")]
it "should combine full spans" $ do
combineSpans
[ Span (Range (Coord 0 0) (Coord 0 2)) "a"
, Span (Range (Coord 0 0) (Coord 0 2)) "b"
]
`shouldBe`
[ (Coord 0 0,"ab")
, (Coord 0 2,"")
]
-- expected: [((Coord 0 0),"ab"),((Coord 0 2),"")]
-- but got: [((Coord 0 0),"a"),((Coord 0 0),"ab"),((Coord 0 2),"b"),((Coord 0 2),"")]
it "should combine three full spans" $ do
combineSpans
[ Span (Range (Coord 0 0) (Coord 0 2)) "a"
, Span (Range (Coord 0 0) (Coord 0 2)) "b"
, Span (Range (Coord 0 0) (Coord 0 2)) "c"
]
`shouldBe`
[ (Coord 0 0,"abc")
, (Coord 0 2,"")
]
-- expected: [((Coord 0 0),"abc"),((Coord 0 2),"")]
-- but got: [((Coord 0 0),"a"),((Coord 0 0),"ab"),((Coord 0 0),"bac"),((Coord 0 2),"cb"),((Coord 0 2),"c"),((Coord 0 2),"")]
it "should combine overlapping spans" $ do
combineSpans
[ Span (Range (Coord 0 0) (Coord 0 3)) "a"
, Span (Range (Coord 0 1) (Coord 0 2)) "b"
]
`shouldBe`
[ (Coord 0 0,"a")
, (Coord 0 1,"ab")
, (Coord 0 2,"a")
, (Coord 0 3,"")
]
-- PASSES !!! |
@jmatsushita This is very close; the current behaviour is that spans are 'inclusive' in their end position, i.e. it "should ignore spans with 0 size ranges" $ do
combineSpans
[ Span (Range (Coord 0 0) (Coord 0 0)) "a"
, Span (Range (Coord 0 0) (Coord 0 0)) "b"
]
`shouldBe`
[
] According to my understanding should instead be: it "should ignore spans with 0 size ranges" $ do
combineSpans
[ Span (Range (Coord 0 0) (Coord 0 0)) "a"
, Span (Range (Coord 0 0) (Coord 0 0)) "b"
]
`shouldBe`
[ (Coord 0 0,"ab")
, (Coord 0 1,"")
] We could of course change the semantics of the Span itself, but I believe the existing work which uses them treats them as inclusive. |
Representing this in ASCII
Where vertical stacking is
mappend
and.
ismempty
Where we would expect:
The text was updated successfully, but these errors were encountered: