Fix an incorrect autocorrect for Capybara/RedundantWithinFind when escape required css selector
#137
Conversation
|
@hatsu38 Does this change solve your issue? |
|
@ydah It seems that RuboCop now properly escapes the within find_by_id("array-form-session.dates") do ~ end-> match within "array-form-session\.dates" do ~ end-> match both So, As mentioned in the ISSUE, the behavior changes as a result of running rubocop -A. Therefore, I believe my issue has not yet been resolved. |
|
For the context: https://stackoverflow.com/questions/12310090/css-selector-with-period-in-id @hatsu38 does the second method, I suggest opening an issue against capybara, as if the backslash quote doesn’t work, it’s in their code. |
pirj
left a comment
pirj
left a comment
There was a problem hiding this comment.
Thank you!
I’m a bit concerned that this change goes above the minimum required to fix the issue, and this involves risks of side effects.
| # css_escape('some-id.with-priod') # => some-id\.with-priod | ||
| # @reference | ||
| # https://github.com/mathiasbynens/CSS.escape/blob/master/css.escape.js | ||
| def css_escape(selector) # rubocop:disable Metrics/AbcSize,Metrics/CyclomaticComplexity,Metrics/MethodLength,Metrics/PerceivedComplexity |
There was a problem hiding this comment.
How do you feel to only add handling for the period in this PR, and leaving the rest for later?
There was a problem hiding this comment.
I updated this PR. we focus on only period for this PR.
|
Thanks for your comment! I tried it again. Originally, I was using And, I found on Stack Overflow that |
ydah
force-pushed
the
fix-136
branch
2 times, most recently
from
afe96d5 to
6a76f00
Compare
…escape required css selector Fix: #136 Previously, the following automatic corrections were made. before: ```ruby within find_by_id("array-form-session.dates") do expect(page).to have_text(:visible, "YYYY/MM/DD") end ``` after: ```ruby within "#array-form-session.dates" do expect(page).to have_text(:visible, "YYYY/MM/DD") end ``` This is `.' in find_by_id. ` has the same meaning as the escaped id. In other words, when replacing within, the `. ` must be escaped. This PR has been modified so that escaping is correctly done in selectors that require escaping. This will prevent the behavior from changing before and after the automatic correction.
|
@hatsu38 I apologize for the delay. I updated this PR. Has this solved your issue? |
Fix: #136
Previously, the following automatic corrections were made.
before:
after:
This is
.infind_by_idhas the same meaning as the escaped id. In other words, when replacing within, the.must be escaped.This PR has been modified so that escaping is correctly done in selectors that require escaping. This will prevent the behavior from changing before and after the automatic correction.
Before submitting the PR make sure the following are checked:
main(if not - rebase it).CHANGELOG.mdif the new code introduces user-observable changes.bundle exec rake) passes (be sure to run this locally, since it may produce updated documentation that you will need to commit).