troessner / reek

Code smell detector for Ruby
https://github.com/troessner/reek
MIT License
4k stars 279 forks source link

Fix editor highlight #1745

Closed JuanVqz closed 8 months ago

JuanVqz commented 8 months ago

Hey, I've noticed my editor is not rendering the code blocks as expected because the first letter is upper case in the code blocks. I wonder if that could be the case for somebody else.

Peek 2023-10-16 16-45

JuanVqz commented 8 months ago

Just noticed it has broken some tests if you are interested in the PR I can fix them otherwise, you can just close it. thanks!

mvz commented 8 months ago

Thanks, @JuanVqz. I think it would be good to make this change. GitHub doesn't seem to care about upper or lower case, but if some editors do care, it's worth changing.

JuanVqz commented 8 months ago

@mvz I'm really glad to have touched those code blocks, I didn't know we had a test that uses our .md files as specs 😃.

https://github.com/JuanVqz/reek/blob/fe0ccaf1718c8b22f7749d09ebed1264f3b7cbca/spec/quality/documentation_spec.rb#L4

certainly, I don't need to explain to you what is happening there because you wrote it but in general, changing the code block from Ruby to ruby makes them executable code and if the block doesn't find the # => it will break because doesn't have anything to assert.

Example 1:

README.md:524

This is really making the tests stop

require 'pry'
binding.pry

Example 2: Subclassed-From-Core-Class.md:11

(rdbg) code
"List = Class.new(Array)\n\nl = List.new\nl << 1\nl << 2\nputs l.reverse.class # => Array"
(ruby)           spec_code = code.gsub(/(^.+) # ?=> (.+$)/, 'assert_equal(\2, \1)')
"List = Class.new(Array)\n\nl = List.new\nl << 1\nl << 2\nassert_equal(Array, puts l.reverse.class)"
(ruby) spec_code
"List = Class.new(Array)\n\nl = List.new\nl << 1\nl << 2\nassert_equal(Array, puts l.reverse.class)"

it seems like the assert doesn't recognize the l variable in its scope

(irb):1:in `eval': (eval):6: syntax error, unexpected local variable or method, expecting `do' or '{' or '(' (SyntaxError)
assert_equal(Array, puts l.reverse.class)

To ignore them just return them to Ruby in upper case.

troessner commented 8 months ago

Merged, thanks @JuanVqz !

troessner commented 8 months ago

Errrrr, can you please squash it to one commit @JuanVqz ? Then I'm happy to merge it.

JuanVqz commented 8 months ago

@troessner and @mvz squashed, thanks!