Wrap comments by word rather than by character #8

petehamilton · 2014-10-09T22:51:15Z

isaacseymour · 2014-10-09T22:56:31Z

lib/atum/generation/erb_context.rb

-          .map { |l| l.scan(/.{1,#{max_line_length}}/) }
-          .flatten.map { |l| starter + l.strip }.join("\n")
+               .flat_map { |l| break_line(l, max_line_length) }
+               .map { |l| starter + l.strip }


Can't these two happen in the one flat_map?

Wrap comments by word rather than by character

isaacseymour · 2014-10-10T09:46:15Z

lib/atum/generation/erb_context.rb

+
+      def break_line(line, max_line_length)
+        line.split.reduce([]) do |lines, word|
+          if lines.empty?


This could be shorter:

if lines.empty? || (lines[-1] + " #{word}").size > max_line_length lines << word else lines[-1] << " #{word}" end lines

Nice. Will update

Already on it

petehamilton force-pushed the comment-wrapping branch from 242e83f to 2b23ff9 Compare October 9, 2014 22:52

isaacseymour reviewed Oct 9, 2014
View reviewed changes

Wrap comments by word rather than by character

6a72afe

petehamilton force-pushed the comment-wrapping branch from 2b23ff9 to 6a72afe Compare October 9, 2014 23:18

petehamilton added a commit that referenced this pull request Oct 9, 2014

Merge pull request #8 from gocardless/comment-wrapping

f9f942d

Wrap comments by word rather than by character

petehamilton merged commit f9f942d into master Oct 9, 2014

petehamilton deleted the comment-wrapping branch October 9, 2014 23:19

isaacseymour reviewed Oct 10, 2014
View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Wrap comments by word rather than by character #8

Wrap comments by word rather than by character #8

petehamilton commented Oct 9, 2014

isaacseymour Oct 9, 2014

isaacseymour Oct 10, 2014

petehamilton Oct 10, 2014

isaacseymour Oct 10, 2014

petehamilton Oct 10, 2014

Wrap comments by word rather than by character #8

Wrap comments by word rather than by character #8

Conversation

petehamilton commented Oct 9, 2014

isaacseymour Oct 9, 2014

Choose a reason for hiding this comment

isaacseymour Oct 10, 2014

Choose a reason for hiding this comment

petehamilton Oct 10, 2014

Choose a reason for hiding this comment

isaacseymour Oct 10, 2014

Choose a reason for hiding this comment

petehamilton Oct 10, 2014

Choose a reason for hiding this comment