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

Virtual Modules and Elm.Arg #88

Merged
merged 120 commits into from
Aug 25, 2024
Merged

Virtual Modules and Elm.Arg #88

merged 120 commits into from
Aug 25, 2024

Conversation

mdgriffith
Copy link
Owner

An API sketch!

That's mostly implemented!

src/Elm.elm Outdated
@@ -10,8 +10,10 @@ module Elm exposing
, Declaration
, comment, declaration
, withDocumentation
, expose, exposeWith
, group
Copy link
Contributor

Choose a reason for hiding this comment

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

I think a withGroup : Declaration -> Declaration could also make sense?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Do you know if you'd need that in your code or if group would work out?

My thinking is that group is going to be pretty great for defining related things all right next to each other, which I already kinda do.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it would be needed, and one can keep using exposeWith in that case?

Thinking more about it, I think that pushing towards having a group close to each other is good

src/Elm.elm Outdated
Comment on lines 176 to 177
done : Fn Expression -> Expression
done (Fn toFnDetails) =
Copy link
Contributor

Choose a reason for hiding this comment

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

I would call this fnDone? And probably the above fnArg.

The reason is that Elm.fnDone is ~clear, Elm.done is not? Am I done with a let? With a function?

)


body : (args -> Expression) -> Fn args -> Expression
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm mildly confused by this? What's the purpose?

src/Elm/Arg.elm Outdated Show resolved Hide resolved
src/Elm/Arg.elm Outdated Show resolved Hide resolved
src/Elm/Declare.elm Show resolved Hide resolved
src/Elm/Declare.elm Outdated Show resolved Hide resolved
(Elm.value
{ importFrom = modName
, name = Format.sanitize name
, annotation = Nothing
Copy link
Contributor

Choose a reason for hiding this comment

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

Here we could grab the type from the Arg, couldn't we?

src/Elm/Declare.elm Outdated Show resolved Hide resolved
{ declaration : Declaration
, call : List Expression -> Expression
, callFrom : List String -> List Expression -> Expression
{ declaration : Elm.Declaration
Copy link
Contributor

Choose a reason for hiding this comment

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

What if you instead accept a Fn tipe and return a Decl tipe? Or something in that vein

src/Elm/Declare.elm Outdated Show resolved Hide resolved

{-| -}
with : Decl required -> Module (required -> val) -> Module val
with decl mod =
Copy link
Contributor

Choose a reason for hiding this comment

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

We also need withUnexposed : Decl optional -> Module val -> Module val I think.

Plus either have with implicitly expose or add an expose/exposeWith : Decl -> Decl

@mdgriffith mdgriffith changed the title Elm.Arg and Declare.elmModule Virtual Modules and Elm.Arg Aug 25, 2024
@mdgriffith mdgriffith merged commit 1c1929c into main Aug 25, 2024
2 checks passed
@mdgriffith mdgriffith deleted the virtual-modules branch August 25, 2024 17:40
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