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

feat: Adds defn- and def- macros #1174

Merged
merged 6 commits into from
Mar 3, 2021
Merged

Conversation

TimDeve
Copy link
Contributor

@TimDeve TimDeve commented Feb 25, 2021

Adding these macros as a shortand for declaring a def or defn and making them hidden and private, useful to keep things internal to a module. Similar to Clojure's defn-.

Adding these macros as a shortand for declaring a def or defn and making
them `hidden` and `private`, useful to keep things internal to a module.
@TimDeve
Copy link
Contributor Author

TimDeve commented Feb 25, 2021

Thinking of adding some example of using hidden & private in the module section of the Language Guide and introduce these two macros at the same time. Does that sound approriate?

@scolsen
Copy link
Contributor

scolsen commented Feb 25, 2021

Thinking of adding some example of using hidden & private in the module section of the Language Guide and introduce these two macros at the same time. Does that sound approriate?

Sounds great! We should add some tests for these too.

@TimDeve
Copy link
Contributor Author

TimDeve commented Feb 25, 2021

Yeah I was thinking about the tests yesterday. Would you expect some "error message" tests?

@TimDeve TimDeve marked this pull request as draft February 25, 2021 17:01
@hellerve
Copy link
Member

Yeah I was thinking about the tests yesterday. Would you expect some "error message" tests?

I’d say so! We can use the test-for-errors setup.

@TimDeve
Copy link
Contributor Author

TimDeve commented Feb 26, 2021

I’d say so! We can use the test-for-errors setup.

Added the tests a few minutes before your message. Doing docs next.

…eGuide

Trying to introduce concepts in the same order they are referred to in
the examples: structs > modules > interfaces.
@TimDeve TimDeve marked this pull request as ready for review February 28, 2021 12:18
@TimDeve
Copy link
Contributor Author

TimDeve commented Feb 28, 2021

Took me weeks to find the time to make this simple PR but I think it's finally ready for review 😄

Copy link
Member

@hellerve hellerve left a comment

Choose a reason for hiding this comment

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

I realized about halfway through that the docs I was annotating weren’t new but rearranged. My comments still hold, I think 😆

@@ -227,6 +156,88 @@ These can only be used at the REPL and during macro evaluation. Here's a subset

To see all functions available in the `Dynamic` module, enter `(info Dynamic)` at the REPL.


### Structs
Any structure type defined in Carp has an init method that can be used to create a new instance. It must be called with all the arguments in the order they are defined.
Copy link
Member

Choose a reason for hiding this comment

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

Could we wrap init in backticks here?

Also, maybe "in the order in which they are defined" or "in definition order" might be better?

(Right [b]))
```

A Variant can be created with the same syntax as call expression:
Copy link
Member

Choose a reason for hiding this comment

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

A variant can be created through a function call?

_ (logic-other)))
```

Note that match works with *values* (not references) takes ownership over the value being matched on. If you instead want to match on a reference, you can use `match-ref`:
Copy link
Member

Choose a reason for hiding this comment

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

I think backticks around match would be good here, and that there is an and missing after the parentheses.

@hellerve
Copy link
Member

I think it would be good to use these forms in stdlib in a follow-up PR, since this could remove a lot of code and test the forms at the same time.

@TimDeve
Copy link
Contributor Author

TimDeve commented Feb 28, 2021

The follow up PR sounds like a good idea. About the comments you've added I was thinking of making a refractor PR for the language guide, the style is not very consistent and some of it seems straight up wrong with the changes in the language since it was written, so I could put your suggested changes it that.

@hellerve
Copy link
Member

The follow up PR sounds like a good idea. About the comments you've added I was thinking of making a refractor PR for the language guide, the style is not very consistent and some of it seems straight up wrong with the changes in the language since it was written, so I could put your suggested changes it that.

Sounds like a good plan! I’ll be happy to help and/or review :)

@TimDeve
Copy link
Contributor Author

TimDeve commented Feb 28, 2021

Once/if this is merged I'll open a draft PR with your suggested changes and we can comment on it on what we add to that.

@eriksvedang
Copy link
Collaborator

Looks good! The changes to the language guide is a bit hard to follow though, has anything been deleted or have things just moved around?

@TimDeve
Copy link
Contributor Author

TimDeve commented Mar 1, 2021

Sorry the merged diff looks really messy, it's easier to follow if you go commit by commit. I moved the module and interface section after the struct section first, and then I added a section about private and hidden in the module section.

@eriksvedang
Copy link
Collaborator

@TimDeve Alright, thanks I'll do that!

Copy link
Collaborator

@eriksvedang eriksvedang left a comment

Choose a reason for hiding this comment

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

@hellerve had some comments that you can choose to address, apart from that I think we're good to go!

@eriksvedang
Copy link
Collaborator

@TimDeve Maybe you already discussed that and this should just go in as-is now?

@TimDeve
Copy link
Contributor Author

TimDeve commented Mar 2, 2021

Yeah the comments where on existing docs so we're going to address them in a different PR.

@eriksvedang eriksvedang merged commit 19c1a4c into carp-lang:master Mar 3, 2021
@eriksvedang
Copy link
Collaborator

Great work!

TimDeve added a commit to TimDeve/Carp that referenced this pull request Apr 7, 2021
@TimDeve TimDeve deleted the private-def branch April 26, 2021 14:09
This pull request was closed.
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.

4 participants