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

Properly correct percent literal delimiters with escape characters in them #2075

Merged
merged 1 commit into from
Jul 29, 2015

Conversation

rrosenblum
Copy link
Contributor

I had an issue where something like %{\n} would be corrected to

%(
)

While trying to fix this, it became easier to replace the delimiter than to figure proper escaping. I am pretty happy with the simplicity of this fix.

@@ -317,5 +317,32 @@
new_source = autocorrect_source(cop, original_source)
expect(new_source).to eq(corrected_source)
end

shared_examples :escape_characters do |percent_literal|
it "corrects #{percent_literal} with \\n as a symbol" do
Copy link
Collaborator

Choose a reason for hiding this comment

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

As a symbol?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops, old naming.

@bbatsov
Copy link
Collaborator

bbatsov commented Jul 28, 2015

Hmm, the changes look reasonable, although I wonder if @jonas054 has something more in mind with the old code.

@jonas054
Copy link
Collaborator

No, I don't think I did. All the old spec examples still pass after this change, and it doesn't look like it's due to poor coverage. It's just that my solution was needlessly complicated. Great work, @rrosenblum!

@@ -39,6 +39,7 @@
* [#2059](https://github.com/bbatsov/rubocop/issues/2059): Don't check for trivial accessors in modules. ([@bbatsov][])
* Add proper punctuation to the end of offense messages, where it is missing. ([@lumeet][])
* [#2071](https://github.com/bbatsov/rubocop/pull/2071): Keep line breaks in place on WordArray autocorrect.([@unmanbearpig][])
* Properly correct `Style/PercentLiteralDelimiters` with escape characters in them. ([@rrosenblum][])
Copy link
Collaborator

Choose a reason for hiding this comment

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

Mention the PR here for completeness.

@rrosenblum
Copy link
Contributor Author

Thanks, it took me a while to realize all that we needed to do was replace the delimiter. I originally was trying to add escaping to the escape characters which was proving difficult.

I have updated the code and rebased.

@bbatsov
Copy link
Collaborator

bbatsov commented Jul 28, 2015

Hmm, the build failed.

@rrosenblum
Copy link
Contributor Author

It looks like the build failed during bundle install. Not sure why that would happen here. I triggered another build so we will see what happens.

@rrosenblum
Copy link
Contributor Author

Build succeeded this time.

@rrosenblum
Copy link
Contributor Author

Speaking of the build, we still have failures in Jruby and RBX.

bbatsov added a commit that referenced this pull request Jul 29, 2015
Properly correct percent literal delimiters with escape characters in them
@bbatsov bbatsov merged commit 24e6e64 into rubocop:master Jul 29, 2015
@bbatsov
Copy link
Collaborator

bbatsov commented Jul 29, 2015

Yeah, we should address them one of these days...

@rrosenblum rrosenblum deleted the fix_percent_literal branch July 29, 2015 15:59
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.

3 participants