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

Add Replace text with named capture groups example. fixes #243 #248

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@Phaiax
Contributor

Phaiax commented Jul 15, 2017

#243

I'm not sure about the category. (I put #242 into basics, but app development seems better now^^)

@Phaiax

This comment has been minimized.

Show comment
Hide comment
@Phaiax

Phaiax Jul 15, 2017

Contributor

I stole this one from the regex crate

Contributor

Phaiax commented Jul 15, 2017

I stole this one from the regex crate

@budziq

This comment has been minimized.

Show comment
Hide comment
@budziq

budziq Jul 15, 2017

Collaborator

Hi @Phaiax thanks for all the submissions!

Unfortunately I'll not be able to follow up with anything resembling review for the next two weeks due to holidays 😞 . I may drop some comments when Im on mobile later and I've asked other maintainers to drop from time to time and look into the PR's.

To answer your unspoken question, regex examples would go to basics section (atm the distinction between basics and app is quite arbitrary and this will change in the future)

Collaborator

budziq commented Jul 15, 2017

Hi @Phaiax thanks for all the submissions!

Unfortunately I'll not be able to follow up with anything resembling review for the next two weeks due to holidays 😞 . I may drop some comments when Im on mobile later and I've asked other maintainers to drop from time to time and look into the PR's.

To answer your unspoken question, regex examples would go to basics section (atm the distinction between basics and app is quite arbitrary and this will change in the future)

@budziq

Hi @Phaiax nice work! just some quick comments from mobile to help out the next reviewer

  • this example would go into basics

also some of the comments in #247 are applicable.

Show outdated Hide outdated src/app.md Outdated
@budziq

Almost perfect! I would only suggest to update PR description with fixes #243 to autoclose the issue on merge.

also I would mention the key identifiers (replace_all) with links and docs about the capture groups.

@Phaiax Phaiax changed the title from Add Replace text with named capture groups example. #243 to Add Replace text with named capture groups example. fixes #243 Jul 23, 2017

@brson

This comment has been minimized.

Show comment
Hide comment
@brson

brson Jul 25, 2017

Contributor

I rebased and merged manually. Thanks @Phaiax

Contributor

brson commented Jul 25, 2017

I rebased and merged manually. Thanks @Phaiax

@brson brson closed this Jul 25, 2017

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