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

Remove mentions to unless/2 in the case/cond/if guide #13852

Merged
merged 2 commits into from
Sep 22, 2024

Conversation

sabiwara
Copy link

I think it is actually a nice simplification since it's one less thing in the getting started.

I'm also trying to rewrite the macro guide which is using unless as an example, I'm not sure what's the best candidate:

  • if: less trivial and a tad verbose because will need to write it in terms of case
  • &&: should be straightforward to change the existing guide, and can easily be implemented in terms of if (but maybe less "interesting/impressive" than implementing if?)
  • keep unless?
  • something else?

@josevalim
Copy link
Member

I think the guide can still use unless and we mention it is an exercise? Although implementing the operator && would also work.

@sabiwara
Copy link
Author

I think the guide can still use unless and we mention it is an exercise?

Yeah that would work, the only part we'd lose is

In fact, unless/2 in Elixir is implemented as a macro:

or perhaps we can say "used to have one but has been deprecated".
But the "impact" might be lesser than showing we're able to implement a first class construct from the language.

@josevalim
Copy link
Member

So let’s go with &&?

@sabiwara
Copy link
Author

I went with a different approach and just changed the conclusion using if: 224b861

The problem with && is to find a good name: if we call this macro_and it would be confusing, and and is not a simple one-liner that can be implemented with an if.

@sabiwara sabiwara merged commit 13cab5f into elixir-lang:main Sep 22, 2024
1 check passed
@sabiwara sabiwara deleted the guides-unless branch September 22, 2024 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants