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

Check for duplicate content in relevant section when inserting into files #735

Merged
merged 1 commit into from
May 31, 2021

Conversation

excid3
Copy link
Contributor

@excid3 excid3 commented Aug 25, 2020

I ran into some unexpected behavior today with insert_into_file. I was trying to insert , null: false into a migration after some text, but nothing happened.

  t.belongs_to :recipient, null: false
  t.string :type
insert_into_file path, ", null: false", after: "t.string :type"

This does nothing because , null: false exists in the file (anywhere).

Using after: I would expect insert_into_file to only care if the string exists after the matching line, not before it.

This PR would change the behavior to only look through the before or after portions of the file content to make it more intuitive.

@rafaelfranca rafaelfranca merged commit 0d3a1dd into rails:master May 31, 2021
@rafaelfranca
Copy link
Member

I'll have to revert this PR since it changes the behavior when the regexp contains capture. split returns the capture inside the array of split strings, so we can't know with the current implementation what is the after and before the regexp.

rafaelfranca added a commit that referenced this pull request Jan 4, 2022
This reverts commit 0d3a1dd, reversing
changes made to ae18824.

This breaks case when the regular expression passed to after and before
contains capture.

Since `String#split` returns the captures inside the array we can't know
with the current implementation what is the before and after the regular
expression.
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