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

Pre 1.3 #241

Merged
merged 35 commits into from
Mar 12, 2015
Merged

Pre 1.3 #241

merged 35 commits into from
Mar 12, 2015

Conversation

cchalmers
Copy link
Member

This is a bunch of miscellaneous changes I wanted to make for 1.3. I've tried to make each commit a separate feature. The main changes are:

Specialise Colour and AlphaColour Color instances to Double

This means you can write fillColour black without a type signature on black. If you happen to be working with a Colour Float you can use colourConvert.

Move (|>) (qualify object with Name) to (>|)

This cleans up space for lenses snoc operator to use with Lines new Cons and Snoc instances.

Remove LineJoinA and LineCapA

I'm not too bothered about this, it just seemed silly to make a newtype around Last just for the instance.

Move view to rectEnvelope

I'm open to name suggestions but I use view a lot from lens and I always forget and get horrible error messages. Also I don't think views a great name because anything outside the box is still visible, it just happens to hide it if you use it on the final diagram.

New show instances

The new show instances hide the internal representation and can be copy pasted to form valid Haskell when possible. Attribute and Style instances show the name of type they're holding which is useful for debugging

Export lens from prelude

It doesn't have to be the whole of lens but I'd like a lot more. Importing lens separately makes a few annoying clashing and it is a prelude after all :)

To clean up the space for lens. I don't view was a great description of
the function anyway.
This is just a replacement for (#) from lens.
And replace the old  function in favour of lenses  and the transform Isos.
This helps with type inference, so if something needs a Color a you can give it green or green `withOpacity` 0.4 and it'll work out it's Double.
Prism' (Texture n) (AlphaColour Double)
Also export Data.Colour.SRGB in Prelude.
For Parametric and EndValues.
Including Cons and Snoc for Lines and _Line and _Loop prisms.
Make a 2D vector from an angle.
There where essensially there for the semigroup instance but took up much more code than just writing the instance out.
For lines, trails, paths and bounding boxes.
Now there's only a few conflicts with lens and Prelude. Namely beside,
backwards, none, transform, (#) (moved to (##)) and (.>). inside and
outside are in Diagrams.BoundingBox but not exported by Prelude.
@cchalmers
Copy link
Member Author

This shouldn't cause many problems with backends. Some of them fail -Werror because of the extra lens exports.

@@ -132,8 +140,11 @@ module Diagrams.Prelude
-- | For working with 'Active' (i.e. animated) things.
, module Data.Active

-- | Essential Lens Combinators
, (&), (.~), (%~)
-- | This exports most of the lens module, excluding the
Copy link
Member

Choose a reason for hiding this comment

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

Should be lens "package" not module. Or maybe it should say "Control.Lens module".

@byorgey
Copy link
Member

byorgey commented Mar 10, 2015

My responses to your main changes are below. I'm still not done looking carefully through all the changes.

Specialise Colour and AlphaColour Color instances to Double

Makes sense.

Move (|>) (qualify object with Name) to (>|)

Also makes sense, though as expressed at diagrams/diagrams-core#82 I am not the biggest fan of the name (>|).

Remove LineJoinA and LineCapA

I'm a pretty big fan of type-directed instances. But in this case perhaps you are right.

Move view to rectEnvelope

Indeed, view is a bad name and it would be good not to conflict with such a basic function from lens. I think rectEnvelope sounds like a good name.

New show instances

Cool.

Export lens from prelude

Makes sense to me.

@cchalmers
Copy link
Member Author

I forgot to mention 72bf032 -- transformation isomorphisms.

I've added transformed, movedTo, movedFrom, translated and rotated Isos and moved under to underT. The new isomorphisms can be used with lens's under. So rotateAround could be implemented as:

rotateAround p angle = under (movedFrom p) (rotate angle)

I've added example usage in the comments for the new Isos.

@bergey
Copy link
Member

bergey commented Mar 10, 2015

I've wanted those transformation Isos for a while. Also the proper Show instance for Angles.

@byorgey
Copy link
Member

byorgey commented Mar 10, 2015

Ah, I like it. I had noticed some code using movedFrom like that and
wondered what was going on. We should make sure to update the part of the
user manual that used to talk about under to mention these.

On 9:48am, Tue, Mar 10, 2015 Chris notifications@github.com wrote:

I forgot to mention 72bf032
72bf032
-- transformation isomorphisms.

I've added transformed, movedTo, movedFrom, translated and rotated Isos
and moved under to underT. The new isomorphisms can be used with lens's
under. So rotateAround could be implemented as:

rotateAround p angle = under (movedFrom p) (rotate angle)

I've added example usage in the comments for the new Isos.


Reply to this email directly or view it on GitHub
#241 (comment).

Conflicts:
	src/Diagrams/Angle.hs
	src/Diagrams/ThreeD/Transform.hs
	src/Diagrams/Transform/Matrix.hs
For the more complicated ones I've just linked the corresponding reverseFoo function.
In 4.6 lens got rid of AReview' and AReview only takes 2 arguments. If we want support for lens <4.6 we need to add some CPP.
@byorgey
Copy link
Member

byorgey commented Mar 11, 2015

Note that lens-4.8 is out now, so we should add support for that as well. I don't know whether we need to support lens < 4.6; probably not.

byorgey and others added 5 commits March 11, 2015 14:41
This only exports things from diagrams or diagrams-lib. This could be usefull want to avoid conflicts other libraries exported from Diagrams.Prelude.

Use Diagrams for Diagrams.Prelude.
These are functions from lens that are likely to conflict with other libraries.
@byorgey
Copy link
Member

byorgey commented Mar 12, 2015

So what's still holding this up? Just deciding what to do with the lens re-exports?

@byorgey
Copy link
Member

byorgey commented Mar 12, 2015

Oh, wait, I see those changes have already been made. This looks good to me.

@jeffreyrosenbluth
Copy link
Member

LGTM as well

@bergey
Copy link
Member

bergey commented Mar 12, 2015

Great!

bergey added a commit that referenced this pull request Mar 12, 2015
@bergey bergey merged commit 0bba344 into master Mar 12, 2015
@byorgey byorgey deleted the pre-1.3 branch February 9, 2017 14:18
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

4 participants