Add LinesIter algorithm to iterate over lines in a geometry

[callpraths]

• I agree to follow the project's code of conduct.
• I added an entry to CHANGES.md if knowledge of this change could be valuable to users.

---

Newly introduced LinesIter is very similar to CoordsIter, but is implemented for only a subset of types (e.g., not for Point, and hence not for Geometry).

This PR also includes a small tweak to the order in which Rect::to_lines() returns the lines -- it now matches the winding order of the polygon created by Rect::to_polygon().

Closes #708

[callpraths]

I would actually like to introduce newtypes for all these concrete LinesIter::Iter types because they currently leak implementation details (about how iterators are chained etc).

I followed the pattern in CoordsIter for this PR, but if there is agreement, I'm happy to introduce newtypes for both CoordsIter and LinesIter.

[callpraths]

I am new(ish) to Rust, and I spent some time trying to wrap my head around this.

My current understanding is that as long as RFC-2071 is unstable, crate APIs should likely not use the impl Trait feature (in the return position, for existential types). Two reasons that I think I'd avoid this feature:

• Clients have no way to store the returned value in a struct / trait associated type (anywhere they must name the type).
• Conditional implementations (#[derive(Debug)] and friends) don't work well). In our case, impl Iterator<Item=Line<T> is not Debug even if both T and Line are).

Basing my understanding so far mostly off of https://www.reddit.com/r/rust/comments/8ik620/notes_on_impl_trait/ and https://users.rust-lang.org/t/embedding-trait-inside-of-struct/37176/14

In this case, I was stuck because of the first issue -- I needed to name the type for LinesIter::Iter, but had no way to do it.

[callpraths]

Did I say I'm new to Rust? Please let me know if I'm understanding this wrong / I'm missing something obvious in how impl Trait works.

[callpraths]

Note to self: Update #681 in some form if impl Trait should be reconsidered in the public API of geo-types.

[michaelkirk]

This was probably oversight. Good catch!

I personally don't think this is the kind of change we'd bump semantic versioning for. Does anyone disagree?

I'm ok to leave "BREAKING" or whatever in the release notes, I'm just loathe to do anything that causes all our downstream crate users to be on an old version.

[callpraths]

I don't have any context on downstream consumers yet, so I don't have a clear opinion here.

I think this falls in the "Possibly-breaking change" per cargo book and in unstable packages, it's OK to bump just the patch version for these changes. The order of returned lines isn't part of an explicit contract, so someone depending on that order to not conform to the winding order is unlikely.

Let me know if I can help make this decision (perhaps start a thread on the channel?)

[michaelkirk]

In the interest of a certain amount of efficiency, I don't expect to get an explicit "OK" on all things. Typically when I'm about to do something potentially controversial, my approach is to make a proposal in the code review and solicit input. If no one complains (we're all async, so I wait a couple days) I go forward.

If it were something that seemed potentially very controversial, I might wait for longer or for a more explicit OK from a larger group of people.

Since no one has yet replied to the contrary, I estimate that it's reasonable to move forward without bumping major version at this point.

[michaelkirk]

The code all LGTM, but I want to leave it unmerged until someone else weighs in on not bumping semver for this change.

[michaelkirk]

bors r+

[bors]

Build succeeded:

• ci result

[callpraths]

Thank you for a great first PR experience!