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 @stack which stacks multiple structs into one. Closes #4 #5

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

tamasgal
Copy link

The @stack macro concatenates the types of the given structs and creates a new one.

Let me know what you think about this. It directly defines the structs, maybe you want it to provide a "template" as an intermediate step first, as proposed in #4 ?

julia> using Mixers

julia> struct Foo
         x::Int32
       end

julia> struct Bar
         y::Int64
       end

julia> @stack Baz Foo Bar

julia> fieldnames(Baz)
(:x, :y)

julia> fieldtypes(Baz)
(Int32, Int64)

@tamasgal
Copy link
Author

Hmm, that ERROR: LoadError: LoadError: UndefVarError: fieldtypes not defined in Julia 1.0 is annoying, not sure how to get around it yet...

@tamasgal
Copy link
Author

Alright, fieldtype is there...

@tamasgal
Copy link
Author

Not sure about the AppVeyor error though, I have no idea about Windows stuff.

@tamasgal
Copy link
Author

I am currently trying to get something like @with_kw @stack Baz Foo Bar to work but no success yet. This is actually something which I would need too 😉

@rafaqz
Copy link
Owner

rafaqz commented Apr 30, 2020

This is cool. Does it handle type parameters on Foo? Probably that and allowing macros like @with_kw would be requirements.

Also maybe @stack needs a dumber name to fit in with the rest of the package ;)

@tamasgal
Copy link
Author

tamasgal commented Apr 30, 2020

Yes I was unsure about the naming, I thought stacks is ok as reference to stacking cups, but no idea 🙈

Here are some ideas:

  • @highball
  • @mash
  • @commonwealth (a cocktail with 71 ingredients)
  • @longisland (not that many ingredients but more popular)
  • @layered reference to layered drinks

Type parameters work like this (I will add tests):

julia> using Mixers, Parameters

julia> struct Foo
                x::Int32
              end

julia> struct Bar
                y::Int64
              end

julia> struct Fjoord{T}
                z::T
              end

julia> @stack Narf{T} Foo Bar Fjoord

julia> Narf{Bool}(1, 2, true)
Narf{Bool}(1, 2, true)

julia> fieldtypes(Narf)
(Int32, Int64, Any)

julia> typeof(Narf{Bool}(1, 2, true))
Narf{Bool}

I however could not get @with_kw accept my struct yet, I tried different quotings but no idea:

julia> using Mixers, Parameters

julia> struct Foo
                x::Int32
              end

julia> struct Bar
                y::Int64
              end

julia> @with_kw @stack Baz Foo Bar
ERROR: LoadError: Only works on type-defs or named tuples.
Make sure to have a space after `@with_kw`, e.g. `@with_kw (a=1,)
Also, make sure to use a trailing comma for single-field NamedTuples.

@rafaqz
Copy link
Owner

rafaqz commented Apr 30, 2020

Oh right sorry I missed that - the outer macro runs first!

The trick is to put that macro inside the @stack macro and reapply it after, like I do here:
https://github.com/rafaqz/Mixers.jl/blob/master/src/Mixers.jl#L37

Try reusing those methods if possible.

Also for consistency with @mix, we should avoid requiring the T here if we can get it from Fjoord

Narf{T}

But the brackets should stay - it marks the paramtric type and sets up the AST to insert T

@rafaqz
Copy link
Owner

rafaqz commented Apr 30, 2020

Also thinking about it I'm not sure how you can get @with_kw to work anyway? it needs the = x?

But allowing macros would allow you to apply some @mix macro inside @stack which would be very cool - you could combine structs and mixins.

@tamasgal
Copy link
Author

Sorry for the late response! I am working on it but it's really tricky, especially that I don't have much experience with macros ;) I'll report back when I got something...

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.

2 participants