Skip to content

Fix inject into file warning#709

Merged
rafaelfranca merged 1 commit into
rails:mainfrom
nicolas-brousse:fix-inject-into-file-warning
May 12, 2023
Merged

Fix inject into file warning#709
rafaelfranca merged 1 commit into
rails:mainfrom
nicolas-brousse:fix-inject-into-file-warning

Conversation

@nicolas-brousse

Copy link
Copy Markdown
Contributor

Not sure if the fix implementation is good enough. Let me know if you want some changes.

Closes #706.

@dorner dorner left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This looks good - I'd like to see this merged in!

@dorner

dorner commented Jun 2, 2021

Copy link
Copy Markdown

@nicolas-brousse can you rebase off master please?

@nicolas-brousse nicolas-brousse force-pushed the fix-inject-into-file-warning branch from 4ae8925 to bf3afab Compare June 3, 2021 10:02
Comment thread lib/thor/actions/inject_into_file.rb Outdated
Comment on lines +115 to +116
before, after = content.split(regexp, 2)
snippet = (behavior == :after ? after : before).to_s

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This code has been added. I've to found a way to deal this regexp.

@dorner dorner left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks good! @rafaelfranca

@nicolas-brousse

Copy link
Copy Markdown
Contributor Author

For now when I tried locally, the test I added fail. I've to inspect how snippet = (behavior == :after ? after : before).to_s works. When I initially worked on this PR this line wasn't implemented yet.

@rafaelfranca

Copy link
Copy Markdown
Member

Tests are broken

@rafaelfranca rafaelfranca force-pushed the fix-inject-into-file-warning branch from bf3afab to 6499e07 Compare May 12, 2023 20:07
@rafaelfranca rafaelfranca merged commit 2dde972 into rails:main May 12, 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.

insert_into_file print warning when change is already done

3 participants