Rubocop: converters #4931

Merged
merged 4 commits into from May 26, 2016

Conversation

Projects
None yet
4 participants
@pathawks
Member

pathawks commented May 24, 2016

No description provided.

@@ -17,19 +18,20 @@ def block_code(code, lang)
Jekyll::External.require_with_graceful_fail("pygments")
lang = lang && lang.split.first || "text"
add_code_tags(
- Pygments.highlight(code, :lexer => lang, :options => { :encoding => 'utf-8' }),
+ Pygments.highlight(code, :lexer => lang, :options => \

This comment has been minimized.

@envygeeks

envygeeks May 24, 2016

Contributor

I think this should just be split up into a multiline hash.

@envygeeks

envygeeks May 24, 2016

Contributor

I think this should just be split up into a multiline hash.

- @renderer.send :include, Redcarpet::Render::SmartyPants if @redcarpet_extensions[:smart]
- markdown = Redcarpet::Markdown.new(@renderer.new(@redcarpet_extensions), @redcarpet_extensions)
+ @redcarpet_extensions[:fenced_code_blocks] = \
+ !@redcarpet_extensions[:no_fenced_code_blocks]

This comment has been minimized.

@envygeeks

envygeeks May 24, 2016

Contributor

What happens if we shorten this class like previously discussed and do something like:

module Parent::NameSpace::Blah
  class NameSpace
  end
end

It seems like something that short really shouldn't be split off. That or we could increase the limit again. Seems like this limit is constantly sabotaging readability.

@envygeeks

envygeeks May 24, 2016

Contributor

What happens if we shorten this class like previously discussed and do something like:

module Parent::NameSpace::Blah
  class NameSpace
  end
end

It seems like something that short really shouldn't be split off. That or we could increase the limit again. Seems like this limit is constantly sabotaging readability.

This comment has been minimized.

@pathawks

pathawks May 24, 2016

Member

That only gets us down to 95 characters 😕

@pathawks

pathawks May 24, 2016

Member

That only gets us down to 95 characters 😕

@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks May 24, 2016

Contributor

Thanks! Just a few comments.

Contributor

envygeeks commented May 24, 2016

Thanks! Just a few comments.

@pathawks pathawks referenced this pull request May 24, 2016

Closed

Make jekyll/jekyll code compliant with rubocop rules #4885

114 of 115 tasks complete
- end
+ def convert(content)
+ @redcarpet_extensions[:fenced_code_blocks] = \
+ !@redcarpet_extensions[:no_fenced_code_blocks]

This comment has been minimized.

@pathawks

pathawks May 24, 2016

Member

Without breaking this up, the line is 97 characters instead of 90 😕

@pathawks

pathawks May 24, 2016

Member

Without breaking this up, the line is 97 characters instead of 90 😕

@pathawks pathawks changed the title from Fix Rubocop warnings in `lib/jekyll/converters` to Rubocop: converters May 24, 2016

@pathawks

This comment has been minimized.

Show comment
Hide comment
Member

pathawks commented May 24, 2016

@pathawks

This comment has been minimized.

Show comment
Hide comment
@pathawks

pathawks May 24, 2016

Member

@envygeeks Is there a reason <figure> is used on line 32 but <div> is used on line 45?

#3779

Member

pathawks commented May 24, 2016

@envygeeks Is there a reason <figure> is used on line 32 but <div> is used on line 45?

#3779

- markdown.render(content)
+ def convert(content)
+ @redcarpet_extensions[:fenced_code_blocks] = \
+ !@redcarpet_extensions[:no_fenced_code_blocks]

This comment has been minimized.

@pathawks

pathawks May 24, 2016

Member

This would be 99 characters if it was all on one line.

@pathawks

pathawks May 24, 2016

Member

This would be 99 characters if it was all on one line.

@@ -100,8 +105,10 @@ def strip_coderay_prefix(hash)
private
def modernize_coderay_config
if highlighter == "coderay"
- Jekyll::Deprecator.deprecation_message "You are using 'kramdown.coderay' in your configuration, " \
+ Jekyll::Deprecator.deprecation_message(
+ "You are using 'kramdown.coderay' in your configuration, " \
"please use 'syntax_highlighter_opts' instead."

This comment has been minimized.

@pathawks

pathawks May 24, 2016

Member

This warning is 101 characters by itself 😳
Perhaps we could use a \n in there?

@pathawks

pathawks May 24, 2016

Member

This warning is 101 characters by itself 😳
Perhaps we could use a \n in there?

This comment has been minimized.

@parkr

parkr May 25, 2016

Member

Yeah, could we just say "kramdown.coderay is deprecated. Use kramdown.syntax_highlighter_opts instead.", or is that still really long?

@parkr

parkr May 25, 2016

Member

Yeah, could we just say "kramdown.coderay is deprecated. Use kramdown.syntax_highlighter_opts instead.", or is that still really long?

@@ -68,8 +71,10 @@ def highlighter
@highlighter = begin
if @config.key?("enable_coderay") && @config["enable_coderay"]
- Jekyll::Deprecator.deprecation_message "You are using 'enable_coderay', " \
+ Jekyll::Deprecator.deprecation_message(
+ "You are using 'enable_coderay', " \
"use syntax_highlighter: coderay in your configuration file."

This comment has been minimized.

@pathawks

pathawks May 24, 2016

Member

This warning is 91 characters by itself 😳
Perhaps we could use a \n in there?

@pathawks

pathawks May 24, 2016

Member

This warning is 91 characters by itself 😳
Perhaps we could use a \n in there?

This comment has been minimized.

@parkr

parkr May 25, 2016

Member

Hehe yeah! Or if you can think of a way to trim down the message.

@parkr

parkr May 25, 2016

Member

Hehe yeah! Or if you can think of a way to trim down the message.

+ Pygments.highlight(
+ code,
+ :lexer => lang,
+ :options => { :encoding => "utf-8" }

This comment has been minimized.

@parkr

parkr May 25, 2016

Member

Could we wrap this hash with { }?

@parkr

parkr May 25, 2016

Member

Could we wrap this hash with { }?

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr May 26, 2016

Member

Wheeeee another one!!! Thank you!!!

@jekyllbot: merge +dev

Member

parkr commented May 26, 2016

Wheeeee another one!!! Thank you!!!

@jekyllbot: merge +dev

@jekyllbot jekyllbot merged commit 8d0a4be into jekyll:master May 26, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

jekyllbot added a commit that referenced this pull request May 26, 2016

@pathawks pathawks deleted the pathawks:converters branch May 26, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment