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.

Sign up for GitHub

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

Jump to bottom

Regex module #60

Closed
wants to merge 2 commits into from
Closed

Regex module #60

wants to merge 2 commits into from

Conversation

eterps
Copy link
Contributor

@eterps eterps commented May 29, 2020 •
edited
Loading

Added Regex module.
It's mostly modeled after: https://package.elm-lang.org/packages/elm/regex/latest/Regex

This resolves gleam-lang/suggestions#40

TODO

  • Add split function
  • Add find function
  • Support submatches
  • Labelled arguments
  • Add replace function
  • Add from_string_with function (re)
  • Documentation
  • Check how to deal with anchors
  • Check how to deal with options
  • Error handling
  • Add replace_first? Add substitute & substitute_first?

@eterps
Copy link
Contributor Author

eterps commented Jun 2, 2020 •
edited
Loading

@lpil I could use some feedback if this is going in the direction you want before I proceed.
Still becoming more familiar with both Gleam & Erlang so feedback on style is also welcome.

lpil
lpil reviewed Jun 3, 2020
View reviewed changes
Copy link
Member

@lpil lpil left a comment

Choose a reason for hiding this comment

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

Looking good! I've left some stylistic notes inline.

src/gleam/regex.gleam Outdated Show resolved Hide resolved
src/gleam/regex.gleam Outdated Show resolved Hide resolved
src/gleam/regex.gleam Outdated Show resolved Hide resolved
src/gleam_stdlib.erl Outdated Show resolved Hide resolved
src/gleam/regex.gleam Outdated Show resolved Hide resolved
@eterps eterps mentioned this pull request Jun 6, 2020
Merged
@lpil lpil closed this Jun 16, 2020
@lpil
Copy link
Member

lpil commented Jun 16, 2020

I updated the main branch and this was closed, I don't understand why. Could you re-open this? Thanks

@eterps
Copy link
Contributor Author

eterps commented Jun 16, 2020 •
edited
Loading

I updated the main branch and this was closed, I don't understand why. Could you re-open this? Thanks

Yeah sure, don't worry. I was thinking to break it up in smaller parts anyway. I like PRs to be small, let me know if you think otherwise (especially the replace/substitute functionality will be a more significant addition).

@lpil
Copy link
Member

lpil commented Jun 16, 2020

Sounds great

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lpil lpil lpil left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Regex library
2 participants
@eterps @lpil