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

Update error message for content already exists case. #799

Merged

Conversation

jpgeek
Copy link
Contributor

@jpgeek jpgeek commented Nov 12, 2022

🌈 The replace! method fails if the flag is not found OR if the text is already present. The error message only says that the flag has not been found. This is incorrect/misleading in the case where it has actually failed because the content is already present. This is pr just adds that to the error message.

Alternatively, could add a separate error message, change the return value from replace! to indicate the type of error but this seems like overkill.

@jpgeek
Copy link
Contributor Author

jpgeek commented Dec 10, 2022

I am also happy to branch the flow to a separate error message for the two cases along with a separate test if that is better.

@rafaelfranca rafaelfranca merged commit 376e141 into rails:main Feb 23, 2023
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.

2 participants