Skip to content

Conversation

AHBruns
Copy link
Contributor

@AHBruns AHBruns commented Oct 6, 2020

Disjunctive patterns are super valuable when you (rightly) don't want to use a wildcard, but have a lot of cases that match to the same thing. I think they should at least be mentioned in the docs even if they may not warrant a whole sub-section.

Because of the tiny size of this change, I just went ahead and opened a PR since it took about the same amount of work as opening an issue on the topic. I hope that's okay.

Disjunctive patterns are super valuable when you (rightly) don't want to use a wildcard, but have a lot of cases that match to the same thing. I think they should at least be mentioned in the docs even if they may not warrant a whole sub-section.
@AHBruns
Copy link
Contributor Author

AHBruns commented Oct 6, 2020

Ah jeez. I'm sorry about triggering so many builds!

@AHBruns
Copy link
Contributor Author

AHBruns commented Oct 6, 2020

I refactored the intro to be simpler, and only mentioned the term disjunctive patterns as a brief aside. I also cleaned up the code examples and added a note about the refactoring advantages (mainly that you can have multiple disjunctive pattern sets unlike wildcards).

This is getting a bit large. So, it might be reasonable to split it into its own sub-section, but I'll leave that decision to you, of course.

@AHBruns AHBruns requested a review from IwanKaramazow October 7, 2020 04:57
@ryyppy
Copy link
Member

ryyppy commented Oct 12, 2020

I like it! Maybe it would make sense to put this content further up in the the "Exhaustiveness Check" Section with a sub header like "Match multiple cases to one expression" or similar? That way it would not get lost in the Tips / Tricks section

@AHBruns
Copy link
Contributor Author

AHBruns commented Oct 12, 2020

Moved it to its own sub-section. LMK if you'd like any other tweaks.

@chenglou
Copy link
Member

Hey @AHBruns. Thanks for the effort; this is indeed valuable to document more explicitly. I'll go ahead and merge it, but I'll rewrite it a bit.

@chenglou chenglou merged commit f5fc090 into rescript-lang:master Oct 14, 2020
@chenglou
Copy link
Member

Done. See my comments at db82e57.

Thanks again!

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