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

Add character start index to function arguments #8

Closed
pejrak opened this issue Nov 16, 2016 · 6 comments
Closed

Add character start index to function arguments #8

pejrak opened this issue Nov 16, 2016 · 6 comments

Comments

@pejrak
Copy link
Contributor

pejrak commented Nov 16, 2016

Hi, I your package helped me in a project where I wanted to wrap fields in a text. Fields were used to replace the matched strings with something else. However, I was missing the information about index of the match start character within the text. I will reference my solution.

pejrak added a commit to pejrak/react-string-replace that referenced this issue Nov 16, 2016
@iansinnott
Copy link
Owner

Cool, thanks @pejrak. This is a good reference for anyone trying to do the same. I'm going to close this out but if there's additional action you'd like to take feel free to reopen.

@pejrak
Copy link
Contributor Author

pejrak commented Nov 16, 2016

Alright, this is probably isolated to my case/scenario. Please consider reviewing/adding if there is anyone else needing that. Thanks for the package and reply.

@iansinnott
Copy link
Owner

Did you want to incorporate this into the library? I didn't know because this is an Issue instead of a Pull Request.

Looking at the official String.prototype.replace docs it does seem that the official function is called with addition args. I would certainly be in support of mirroring that API since it's consistent with JS.

Maybe you could make a PR with your changes and we can discuss there?

@iansinnott iansinnott reopened this Nov 17, 2016
@pejrak
Copy link
Contributor Author

pejrak commented Nov 22, 2016

Ian, I had other commitments last week, but I just added a pull request to my fork.

Let me see if I can write a test on this.

@pejrak
Copy link
Contributor Author

pejrak commented Nov 22, 2016

I suppose this is the offset from String.prototype.replace docs

Test added.

iansinnott added a commit that referenced this issue Nov 22, 2016
Added character start to fn call arguments per issue #8
@iansinnott
Copy link
Owner

Published in v0.4. Thanks for the help on this one @pejrak

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

No branches or pull requests

2 participants