Lucid

[jeffreyrosenbluth]

Ready to Merge

This branch is a port of diagrams-svg to use lucid-svg, instead of blaze-svg.
I wrote the lucid-svg library after reading Chris Done's blog posts http://chrisdone.com/posts/lucid
and http://chrisdone.com/posts/lucid2, which do a good job of explaining why he created Lucid as
an alternative to blaze-html. Most of the advantages he discusses there, apply equally to lucid-svg.
But to summarize we get

• No name conflicts with base allowing us to use Lucid.Svg unqualified
• Consistent naming for elements and attributes, and no name clashes do to polymorphism.
• A real monad instance
• Better syntax and composition of attributes
• A single package to import
• Monoidal and monadic interface

In addition, we get complete control over lucid-svg (although, I suspect that since byorgey was the most
recent uploader of blaze-svg, we probably have pretty good control there too). and a small lightweight
framework.

The disadvantages are,

• Blaze is slightly faster
• diagrams-svg using blaze has been battle tested

Please let me know your thoughts and if you get a chance run it through some test diagrams.

@bergey , @byorgey , @fryguybob , @cchalmers

[cchalmers]

Looks go so far. I have a few small notes:

• some T.concat . map can be replaced with foldMap
• I'm not a fan of using the Show instance for numbers -- it's unnecessary precise and I some (older mac os's) don't parse exponential form. You could use Data.Text.Lazy.Builder.RealFloat or Numeric to get fixed precision and lose the Show constraint.
• This seems like a nice opportunity to add the Opacity Annotation for group opacity, it should be pretty similar to how Href is done.

[jeffreyrosenbluth]

@cchalmers Thanks for taking a look.

• T.concat . map to `foldMap
• I'd rather think about replacing the show instance for numbers more broadly and not included it
  in this PR, since it is an issue in more than one of our diagrams libraries.

For the Opacity Annotation do you mean something like:

fromRTree (Node (RAnnot (OpacityGroup o)) rs)
        = R $ do
            let R r =  foldMap fromRTree rs
            svg <- r
            return $ g_ [opacity_ $ toText o] svg

[cchalmers]

Yeah, that looks about right.

[jeffreyrosenbluth]

Currently I cannot create an analogous Hashable instance for SVGOptions since
the constructor for Attribute is not exposed from lucid. I'm not even sure exactly how we use
the hashable instance for, I'm wondering if we will still need it after 'new builder'?
If so is there a work around (other than changing the type of _svgDefinintions) or should I ask Chris Done to expose the constructor. He has been
very responsive to all of my requests so far?

[cchalmers]

Yeah, still need the Hashable instance. It's just so we can check if the diagram has changed. You could write a bogus instance and we just won't detect changes to the definitions. I think it's still worth asking to Chris to expose the internals.

[jeffreyrosenbluth]

Thanks, I'll ask him

[cchalmers]

On a semi-related note, do you know why Chris chose to use blaze builder? I was under the impression the new bytestring-builder was faster.

[jeffreyrosenbluth]

I don't know why he chose blaze, there is nothing about it in either of the two blog posts he wrote about lucid.

[jeffreyrosenbluth]

At some point we should probably just write our own version of something like Lucid.Base specialized to SVG, it's easy enough.

[cchalmers]

Thanks. I tried it and there's a modest speed increase in his benchmarks so I'll submit a PR and see what he says.

[jeffreyrosenbluth]

cool

[chrisdone]

On a semi-related note, do you know why Chris chose to use blaze builder? I was under the impression the new bytestring-builder was faster.

I hadn't heard about the new bytestring-builder at the time of writing.

[chrisdone]

Pushed to hackage as 2.7.0.

[chrisdone]

Hm, I think I jumped the gun. Is there a way to convert from bytestring-builder to blaze-builder? E.g. Yesod expects a blaze-builder. If not this is likely to break people's code in a way with no clear migration path.

[chrisdone]

I've reverted this bump as 2.8.0 until I can provide a clear migration path. Sorry. I'd naively assumed there would be a migration path from these two types that users could follow. See also meiersi/blaze-builder#17

[chrisdone]

I've bumped 2.8.1 with the Attribute constructor exported, for what it's worth.

[cchalmers]

You can get there by using ByteString as an intermediate but I don't know efficient it is.

I've bumped 2.8.1 with the Attribute constructor exported, for what it's worth.

Thanks :)

[jeffreyrosenbluth]

To do before merge:

• Test, including backend-tests
• Pretty printing

[chrisdone]

Seems that we'll have a migration path for Builder at some point soon: meiersi/blaze-builder#17 (comment)

[jeffreyrosenbluth]

I think this is ready to merge.
Provided everyone is happy to make the switch.

[cchalmers]

I'm happy to merge.

[jeffreyrosenbluth]

Anyone else have an opinion before we merge?

[fryguybob]

Looks good to me.

[jeffreyrosenbluth]

Cool, would one of you mind doing the merge.

On Wed Jan 28 2015 at 6:42:35 AM Ryan Yates [email protected]
wrote:

Looks good to me.

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