[#8827] Change Style/FormatStringToken style to template #10723
Conversation
pirj
force-pushed
the
8227-change-enforced-stye-of-format-string-token-cop
branch
from
63151e3
to
02a7856
Compare
pirj
changed the title
pirj
force-pushed
the
8227-change-enforced-stye-of-format-string-token-cop
branch
2 times, most recently
from
0c1c281
to
9bb0f07
Compare
|
While I don't mind changing this for RuboCop itself (unless someone from @rubocop/rubocop-core objects), that's a breaking configuration change we can't do before 2.0. |
pirj
force-pushed
the
8227-change-enforced-stye-of-format-string-token-cop
branch
from
9bb0f07
to
765bca1
Compare
Fair enough. The only way to push such changes through quicker is to release major versions more often. And this is precisely what I'm trying to do by squashing the remaining tickets for the 2.0 milestone. My primary concern is that it's been more than 1.5 years since RuboCop 1.0 release. Presently, there are 60 cops in If my understanding that it's better to release 2.0 soon is correct, I intend to continue with the few remaining 2.0 issues. |
pirj
force-pushed
the
8227-change-enforced-stye-of-format-string-token-cop
branch
from
765bca1
to
ea90ff3
Compare
| @@ -146,7 +146,7 @@ def highlighted_area | |||
| # @api private | |||
| # This is just for debugging purpose. | |||
| def to_s | |||
| format('%<severity>s:%3<line>d:%3<column>d: %<message>s', | |||
There was a problem hiding this comment.
The following change leads to a build failure on JRuby:
- format('%<severity>s:%3<line>d:%3<column>d: %<message>s',
+ format('%{severity}:%3{line}:%3{column}: %{message}', expected: "C: 1: 1: message % test"
got: "C:1:1: message % test"
Works fine on MRI. Even though it probably makes sense to file a JRuby ticket, I'd say that we have to keep this as is to support correct behaviour on older JRuby versions.
cc @headius
| @@ -46,7 +46,7 @@ def report_file(file, offenses) | |||
|
|
|||
| offenses.each do |o| | |||
| output.printf( | |||
| "%<severity>s:%3<line>d:%3<column>d: %<message>s\n", | |||
| CONTRIBUTOR = '[@%<user>s]: https://github.com/%<user>s' | ||
| SIGNATURE = Regexp.new(format(Regexp.escape('[@%<user>s][]'), user: '([\w-]+)')) | ||
| CONTRIBUTOR = '[@%{user}]: https://github.com/%{user}' | ||
| SIGNATURE = Regexp.new(format(Regexp.escape('[@%<user>s][]'), user: '([\w-]+)')) # rubocop:disable Style/FormatStringToken |
There was a problem hiding this comment.
Appreciate any clever solution that wouldn't offend the cop and would allow for Regexp nesting with escaping. I cracked my teeth here.
| MSG = 'Assignment Branch Condition size for %<method>s is too high. ' \ | ||
| '[%<abc_vector>s %<complexity>.4g/%<max>.4g]' | ||
| MSG = 'Assignment Branch Condition size for %{method} is too high. ' \ | ||
| '[%{abc_vector} %<complexity>0.4g/%<max>0.4g]' # rubocop:disable Style/FormatStringToken |
There was a problem hiding this comment.
It feels that the cop needs to consider this as an exception:
format('%<complexity>.4g', complexity: 12.3456) # => "12.35"
format('%{complexity}.4g', complexity: 12.3456) # => "12.3456.4g"
I'm not opposed to doing major releases more often in general. We should probably start some discussion about our goals for 2.0 soon. |
|
Does it make sense to make a
|
pirj
force-pushed
the
8227-change-enforced-stye-of-format-string-token-cop
branch
from
ea90ff3
to
fbf9518
Compare
pirj
force-pushed
the
8227-change-enforced-stye-of-format-string-token-cop
branch
from
fbf9518
to
ddb695b
Compare
|
We can just create a branch named |
I think it's better NOT to make |
fixes #8827
templateseems to be by far the most accepted style, at least in thereal-world-rspecrepositories.Before submitting the PR make sure the following are checked:
[Fix #issue-number](if the related issue exists).master(if not - rebase it).bundle exec rake default. It executes all tests and runs RuboCop on its own code.{change_type}_{change_description}.mdif the new code introduces user-observable changes. See changelog entry format for details.