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

Graphics model #89

Closed
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@laszlopandy
Contributor

laszlopandy commented Jan 10, 2015

  • Including Debug will not include Collage or Element
  • Public types are re-exported from the same module as before.
  • Private types which were exposed before are now hidden: BasicForm, ShapeStyle, FillStyle, ElementPrim, ImageStyle, Three.
  • Since no one should have been using the private types, I don't expect any breaking changes from the Graphics.Model refactor.
type alias Shape = List (Float,Float)
{-| The shape of the ends of a line. -}
type LineCap = Flat | Round | Padded

This comment has been minimized.

@evancz

evancz Jan 11, 2015

Member

In previous versions, these constructors were exposed. Now there is no way to construct something of type LineCap. I worry that we are doing bad changes as a stopgap for the real answer here.

Another way to get the same benefit in code size is to write a crash function in Native.Signal and then we don't have to do any of this. Or move Debug.crash into its own thing separate from Debug.trace

@evancz

evancz Jan 11, 2015

Member

In previous versions, these constructors were exposed. Now there is no way to construct something of type LineCap. I worry that we are doing bad changes as a stopgap for the real answer here.

Another way to get the same benefit in code size is to write a crash function in Native.Signal and then we don't have to do any of this. Or move Debug.crash into its own thing separate from Debug.trace

This comment has been minimized.

@laszlopandy

laszlopandy Jan 11, 2015

Contributor

I'm in favour of splitting Debug into log + crash and the other time
travelling debugger functions.

But that would mean that Html would still pull in Collage and Graphics.

On Sunday, January 11, 2015, Evan Czaplicki notifications@github.com
wrote:

In src/Graphics/Model.elm
https://github.com/elm-lang/core/pull/89#discussion-diff-22767472:

  • , form : BasicForm
  • }

+type BasicForm

  • = FPath LineStyle Path
  • | FShape ShapeStyle Shape
  • | FImage Int Int (Int,Int) String
  • | FElement Element
  • | FGroup Transform2D (List Form)

+type alias Path = List (Float,Float)
+
+type alias Shape = List (Float,Float)
+
+{-| The shape of the ends of a line. -}
+type LineCap = Flat | Round | Padded

In previous versions, these constructors were exposed. Now there is no way
to construct something of type LineCap. I worry that we are doing bad
changes as a stopgap for the real answer here.

Another way to get the same benefit in code size is to write a crash
function in Native.Signal and then we don't have to do any of this. Or
move Debug.crash into its own thing separate from Debug.trace


Reply to this email directly or view it on GitHub
https://github.com/elm-lang/core/pull/89/files#r22767472.

@laszlopandy

laszlopandy Jan 11, 2015

Contributor

I'm in favour of splitting Debug into log + crash and the other time
travelling debugger functions.

But that would mean that Html would still pull in Collage and Graphics.

On Sunday, January 11, 2015, Evan Czaplicki notifications@github.com
wrote:

In src/Graphics/Model.elm
https://github.com/elm-lang/core/pull/89#discussion-diff-22767472:

  • , form : BasicForm
  • }

+type BasicForm

  • = FPath LineStyle Path
  • | FShape ShapeStyle Shape
  • | FImage Int Int (Int,Int) String
  • | FElement Element
  • | FGroup Transform2D (List Form)

+type alias Path = List (Float,Float)
+
+type alias Shape = List (Float,Float)
+
+{-| The shape of the ends of a line. -}
+type LineCap = Flat | Round | Padded

In previous versions, these constructors were exposed. Now there is no way
to construct something of type LineCap. I worry that we are doing bad
changes as a stopgap for the real answer here.

Another way to get the same benefit in code size is to write a crash
function in Native.Signal and then we don't have to do any of this. Or
move Debug.crash into its own thing separate from Debug.trace


Reply to this email directly or view it on GitHub
https://github.com/elm-lang/core/pull/89/files#r22767472.

This comment has been minimized.

@evancz

evancz Jan 11, 2015

Member

So far, this PR cannot address the latter though because the public documentation will not work out. Why should we do wrong things as stopgaps, especially if they do not really work? I am okay with some extra code size for a little while compared to the changes we are considering here.

@evancz

evancz Jan 11, 2015

Member

So far, this PR cannot address the latter though because the public documentation will not work out. Why should we do wrong things as stopgaps, especially if they do not really work? I am okay with some extra code size for a little while compared to the changes we are considering here.

@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Jan 11, 2015

Member

Big question: why is it important and valuable to do this instead of focusing on dead code elimination? Long term, that will be implemented, and it seems like these changes are not desirable in that world.

Member

evancz commented Jan 11, 2015

Big question: why is it important and valuable to do this instead of focusing on dead code elimination? Long term, that will be implemented, and it seems like these changes are not desirable in that world.

@laszlopandy

This comment has been minimized.

Show comment
Hide comment
@laszlopandy

laszlopandy Jan 11, 2015

Contributor

Yes. I've said before: dead code elimination enables good API design.
So let's drop this, but remember to keep DCE on the roadmap.

Contributor

laszlopandy commented Jan 11, 2015

Yes. I've said before: dead code elimination enables good API design.
So let's drop this, but remember to keep DCE on the roadmap.

@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Jan 11, 2015

Member

Sounds good, it should be really fun to work on that :)

Member

evancz commented Jan 11, 2015

Sounds good, it should be really fun to work on that :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment