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

Add some tests for render/view #356

Merged
merged 3 commits into from
Aug 31, 2014
Merged

Add some tests for render/view #356

merged 3 commits into from
Aug 31, 2014

Conversation

erbridge
Copy link
Contributor

The single cursor tests are failing, probably due to a bug in text.Region.Intersection.
ViewRegionMap still needs testing.

The single cursor tests are failing, probably due to a bug in text.Region.Intersection.
ViewRegionMap still needs testing.
@quarnster
Copy link
Member

view_test.go:105: Test 1: Expected [(100, 100)], but got [(0, 0)]
view_test.go:105: Test 2: Expected [(100, 100)], but got []
view_test.go:105: Test 3: Expected [(100, 100)], but got []
view_test.go:105: Test 4: Expected [(150, 150)], but got []
view_test.go:105: Test 5: Expected [(100, 100)], but got []

They should all be [] because there's nothing to render for empty regions. (Cursors are handled elsewhere).

text.Region.Intersection is actually behaving correctly, try running this in the ST3 console: a = sublime.Region(100,100); b = sublime.Region(100,100); a.intersection(b) and the output is (0, 0).

render.ViewRegions.Cull should check the returned intersection region if it's empty before adding it to fix test 1.

@erbridge
Copy link
Contributor Author

That seems a bit weird to me. The intersection of the two regions is the cursor. I can understand that we'd want to exclude them, though, so although I still think text.Region.Intersection is misbehaving, I'll fix these tests.

@quarnster
Copy link
Member

IMO you are correct regarding Intersection, especially as intersects returns true for the same input, but let's not fix it until it proves to be a problem simply because it's what ST3 does.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.91%) when pulling f6e0bcd on erbridge:render_view_test into f349b59 on limetext:master.

erbridge added a commit that referenced this pull request Aug 31, 2014
@erbridge erbridge merged commit bfb2484 into limetext:master Aug 31, 2014
@erbridge erbridge deleted the render_view_test branch August 31, 2014 11:30
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