From 6470ebf30f4b54bfad139670299dc450f98045b9 Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Sat, 30 Sep 2017 20:44:11 +0200 Subject: [PATCH 01/43] Add line length restrained printing utilities --- lib/rufo.rb | 2 + lib/rufo/doc_builder.rb | 123 +++++++++++++ lib/rufo/doc_printer.rb | 289 ++++++++++++++++++++++++++++++ spec/lib/rufo/doc_builder_spec.rb | 25 +++ spec/lib/rufo/doc_printer_spec.rb | 100 +++++++++++ 5 files changed, 539 insertions(+) create mode 100644 lib/rufo/doc_builder.rb create mode 100644 lib/rufo/doc_printer.rb create mode 100644 spec/lib/rufo/doc_builder_spec.rb create mode 100644 spec/lib/rufo/doc_printer_spec.rb diff --git a/lib/rufo.rb b/lib/rufo.rb index 34cdec25..63a634d4 100644 --- a/lib/rufo.rb +++ b/lib/rufo.rb @@ -14,5 +14,7 @@ def self.format(code, **options) require_relative "rufo/command" require_relative "rufo/dot_file" require_relative "rufo/settings" +require_relative "rufo/doc_builder" +require_relative "rufo/doc_printer" require_relative "rufo/formatter" require_relative "rufo/version" diff --git a/lib/rufo/doc_builder.rb b/lib/rufo/doc_builder.rb new file mode 100644 index 00000000..69df8494 --- /dev/null +++ b/lib/rufo/doc_builder.rb @@ -0,0 +1,123 @@ +module Rufo + class DocBuilder + class InvalidDocError < StandardError; end + + class << self + + # Combine an array of items into a single string. + def concat(parts) + assert_docs(parts) + { + type: :concat, parts: parts, + } + end + + # Increase level of indentation. + def indent(contents) + assert_doc(contents) + { + type: :indent, contents: contents, + } + end + + # Increase indentation by a fixed number. + def align(n, contents) + assert_doc(contents) + {type: :align, contents: contents, n: n} + end + + # Groups are items that the printer should try and fit onto a single line. + # If the group does not fit then it breaks instead. + def group(contents, opts = {}) + assert_doc(contents) + { + type: :group, + contents: contents, + break: !!opts[:should_break], + expanded_states: opts[:expanded_states], + } + end + + # Rather than breaking if the items do not fit this tries a different set + # of items. + def conditional_group(states, opts = {}) + group(states.first, opts.merge(expanded_states: states)) + end + + # Alternative to group. This only breaks the required items rather then + # all items if they do not fit. + def fill(parts) + assert_docs(parts) + + {type: :fill, parts: parts} + end + + # Print first arg if the group breaks otherwise print the second. + def if_break(break_contents, flat_contents) + assert_doc(break_contents) if break_contents.present? + assert_doc(flat_contents) if flat_contents.present? + + {type: :if_break, break_contents: break_contents, flat_contents: flat_contents} + end + + # Append content to the end of a line. This gets placed just before a new line. + def line_suffix(contents) + assert_doc(contents) + {type: :line_suffix, contents: contents} + end + + # Join list of items with a separator. + def join(sep, arr) + result = [] + arr.each_with_index do |element, index| + unless index == 0 + result << sep + end + result << element + end + concat(result) + end + + def add_alignment_to_doc(doc, size, tab_width) + return doc unless size > 0 + (size/tab_width).times { doc = indent(doc) } + doc = align(size % tab_width, doc) + align(-Float::INFINITY, doc) + end + + private + + def assert_docs(parts) + parts.each(&method(:assert_doc)) + end + + def assert_doc(val) + unless val.is_a?(String) || (val.is_a?(Hash) && val[:type].is_a?(Symbol)) + raise InvalidDocError.new("Value #{val.inspect} is not a valid document") + end + end + end + + # Use this to ensure that line suffixes do not move to the last line in group. + LINE_SUFFIX_BOUNDARY = {type: :line_suffix_boundary} + # Use this to force the parent to break + BREAK_PARENT = {type: :break_parent} + # If the content fits on one line the newline will be replaced with a space. + # Newlines are what triggers indentation to be added. + LINE = {type: :line} + # If the content fits on one line the newline will be replaced by nothing. + SOFT_LINE = {type: :line, soft: true} + # This newline is always included regardless of if the content fits on one + # line or not. + HARD_LINE = concat([{type: :line, hard: true}, BREAK_PARENT]) + # This is a newline that is always included and does not cause the + # indentation to change subsequently. + LITERAL_LINE = concat([ + {type: :line, hard: true, literal: true}, + BREAK_PARENT, + ]) + + # This keeps track of the cursor in the document. + CURSOR = {type: :cursor, placeholder: :cursor} + end +end diff --git a/lib/rufo/doc_printer.rb b/lib/rufo/doc_printer.rb new file mode 100644 index 00000000..f3d086cf --- /dev/null +++ b/lib/rufo/doc_printer.rb @@ -0,0 +1,289 @@ +module Rufo + class DocPrinter + ROOT_INDENT = { + indent: 0, + align: { + spaces: 0, + }, + } + MODE_BREAK = 1 + MODE_FLAT = 2 + INDENT_WIDTH = 2 + class << self + def print_doc_to_string(doc, opts) + width = opts.fetch(:print_width) + new_line = opts.fetch(:new_line, "\n") + pos = 0 + + cmds = [[ROOT_INDENT, MODE_BREAK, doc]] + out = [] + should_remeasure = false + line_suffix = [] + + while cmds.length != 0 + x = cmds.pop + puts x.inspect + ind = x[0] + mode = x[1] + doc = x[2] + if doc.is_a?(String) + out.push(doc) + pos += doc.length + else + puts doc.inspect + case doc[:type] + when :cursor + out.push(doc[:placeholder]) + when :concat + doc[:parts].reverse_each { |part| cmds.push([ind, mode, part]) } + when :indent + cmds.push([make_indent(ind), mode, doc[:contents]]) + when :align + cmds.push([make_align(ind, doc[:n]), mode, doc[:contents]]) + when :group + if mode == MODE_FLAT && !should_remeasure + cmds.push([ind, doc[:break] ? MODE_BREAK : MODE_FLAT, doc[:contents]]) + next + end + if mode == MODE_FLAT || mode == MODE_BREAK + should_remeasure = false + next_cmd = [ind, MODE_FLAT, doc[:contents]] + rem = width - pos + + if !doc[:break] && fits(next_cmd, cmds, rem) + cmds.push(next_cmd) + else + unless doc[:expanded_states].nil? + most_expanded = doc[:expanded_states].last + + if doc[:break] + cmds.push([ind, MODE_BREAK, most_expanded]) + next + else + best_state = doc[:expanded_states].find { |state| + state_cmd = [ind, MODE_FLAT, state] + fits(state_cmd, cmds, rem) + } || most_expanded + cmds.push([ind, MODE_FLAT, best_state]) + end + else + cmds.push([ind, MODE_BREAK, doc[:contents]]) + end + end + end + when :fill + rem = width - pos + parts = doc[:parts] + next if parts.empty? + + content = parts[0] + contents_flat_cmd = [ind, MODE_FLAT, content] + contents_break_cmd = [ind, MODE_BREAK, content] + content_fits = fits(contents_flat_cmd, [], width - rem, true) + if parts.length == 1 + if content_fits + cmds.push(contents_flat_cmd) + else + cmds.push(contents_break_cmd) + end + + next + end + + whitespace = parts[1] + whitespace_flat_cmd = [ind, MODE_FLAT, whitespace] + whitespace_break_cmd = [ind, MODE_BREAK, whitespace] + if parts.length == 2 + if content_fits + cmds.push(whitespace_flat_cmd) + cmds.push(contents_flat_cmd) + else + cmds.push(whitespace_break_cmd) + cmds.push(contents_break_cmd) + end + next + end + + remaining = parts[2..-1] + remaining_cmd = [ind, mode, DocBuilder::fill(remaining)] + + second_content = parts[2] + first_and_second_content_flat_cmd = [ + ind, MODE_FLAT, DocBuilder::concat([content, whitespace, second_content]), + ] + first_and_second_content_fits = fits( + first_and_second_content_flat_cmd, + [], + rem, + true + ) + + if first_and_second_content_fits + cmds.push(remaining_cmd) + cmds.push(whitespace_flat_cmd) + cmds.push(contents_flat_cmd) + elsif content_fits + cmds.push(remaining_cmd) + cmds.push(whitespace_break_cmd) + cmds.push(contents_flat_cmd) + else + cmds.push(remaining_cmd) + cmds.push(whitespace_break_cmd) + cmds.push(contents_break_cmd) + end + when :if_break + if mode === MODE_BREAK + if doc[:break_contents] + cmds.push([ind, mode, doc[:break_contents]]) + end + end + if mode === MODE_FLAT + if doc[:flat_contents] + cmds.push([ind, mode, doc[:flat_contents]]) + end + end + when :line_suffix + line_suffix.push([ind, mode, doc[:contents]]) + when :line_suffix_boundary + if line_suffix.length > 0 + cmds.push([ind, mode, {type: :line, hard: true}]) + end + when :line + if mode == MODE_FLAT + unless doc[:hard] + unless doc[:soft] + out.push(" ") + pos += 1 + end + next + else + should_remeasure = true + end + end + if mode == MODE_FLAT || mode == MODE_BREAK + unless line_suffix.empty? + cmds.push([ind, mode, doc]) + cmds.concat(line_suffix.reverse) + line_suffix = [] + next + end + + if doc[:literal] + out.push(new_line) + pos = 0 + else + if out.length > 0 + while out.length > 0 && out.last.match?(/^[^\S\n]*$/) + out.pop + end + + unless out.empty? + out[-1] = out.last.sub(/[^\S\n]*$/, "") + end + end + + length = ind[:indent] * INDENT_WIDTH + ind[:align][:spaces] + indent_string = " " * length + out.push(new_line, indent_string) + pos = length + end + end + end + end + end + + cursor_place_holder_index = out.index(DocBuilder::CURSOR[:placeholder]) + + if cursor_place_holder_index + before_cursor = out[0..(cursor_place_holder_index-1)].join("") + after_cursor = out[(cursor_place_holder_index+1)..-1].join("") + + return { + formatted: before_cursor + after_cursor, + cursor: before_cursor.length, + } + end + + {formatted: out.join("")} + end + + private + + def make_indent(ind) + { + indent: ind[:indent] + 1, + align: ind[:align], + } + end + + def make_align(ind, n) + return ROOT_INDENT if n == -Float::INFINITY + { + indent: ind[:indent], + align: { + spaces: ind[:align][:spaces] + n, + }, + } + end + + def fits(next_cmd, rest_cmds, width, must_be_flat = false) + rest_idx = rest_cmds.size + cmds = [next_cmd] + + while width >= 0 + if cmds.size == 0 + return true if (rest_idx == 0) + cmds.push(rest_cmds[rest_idx - 1]) + + rest_idx -= 1 + next + end + + x = cmds.pop + ind = x[0] + mode = x[1] + doc = x[2] + + if doc.is_a?(String) + width -= doc.length + else + case doc[:type] + when :concat + doc[:parts].each { |part| cmds.push([ind, mode, part]) } + when :indent + cmds.push(make_indent(ind), mode, doc[:contents]) + when :align + cmds.push([make_align(ind, doc[:n]), mode, doc[:contents]]) + when :group + return false if must_be_flat && doc[:break] + cmds.push([ind, doc[:break] ? MODE_BREAK : mode, doc[:contents]]) + when :fill + doc[:parts].each { |part| cmds.push([ind, mode, part]) } + when :if_break + if mode == MODE_BREAK && doc[:break_contents] + cmds.push([ind, mode, doc[:break_contents]]) + elsif mode == MODE_FLAT && doc[:flat_contents] + cmds.push([ind, mode, doc[:flat_contents]]) + end + when :line + case mode + when MODE_FLAT + unless doc[:hard] + unless doc[:soft] + width -= 1 + end + else + return true + end + when MODE_BREAK + return true + end + end + end + end + + return false + end + end + end +end diff --git a/spec/lib/rufo/doc_builder_spec.rb b/spec/lib/rufo/doc_builder_spec.rb new file mode 100644 index 00000000..958766aa --- /dev/null +++ b/spec/lib/rufo/doc_builder_spec.rb @@ -0,0 +1,25 @@ +require 'spec_helper' + +RSpec.describe Rufo::DocBuilder do + it 'allows valid documents to be created' do + parts = ["something", {type: :symbol_yo}] + expect(described_class.concat(parts)).to eql({ + type: :concat, + parts: parts, + }) + end + + it 'raises an error if a document is not the correct type' do + invalid_docs = [ + [], + 1, + {}, + {type: "string"}, + ] + invalid_docs.each do |doc| + expect { described_class.concat([doc]) }.to raise_error( + Rufo::DocBuilder::InvalidDocError + ) + end + end +end diff --git a/spec/lib/rufo/doc_printer_spec.rb b/spec/lib/rufo/doc_printer_spec.rb new file mode 100644 index 00000000..6d110243 --- /dev/null +++ b/spec/lib/rufo/doc_printer_spec.rb @@ -0,0 +1,100 @@ +require 'spec_helper' + +RSpec.describe Rufo::DocPrinter do + B = Rufo::DocBuilder + + def print(doc, options = nil) + print_doc(doc, options)[:formatted] + end + + def print_doc(doc, options = nil) + options ||= {} + options = {print_width: 80}.merge!(options) + described_class.print_doc_to_string(doc, options) + end + + it 'prints concatenations' do + doc = B.concat(["a", " = ", "1"]) + expect(print(doc)).to eql("a = 1") + end + + it 'prints lines' do + doc = B.group(B.concat(["asd", B::LINE, "123"])) + expect(print(doc)).to eql("asd 123") + expect(print(doc, print_width: 4)).to eql("asd\n123") + expect(print(doc, print_width: 2)).to eql("asd\n123") + end + + it 'prints soft lines' do + doc = B.group(B.concat(["asd", B::SOFT_LINE, "123"])) + expect(print(doc)).to eql("asd123") + expect(print(doc, print_width: 4)).to eql("asd\n123") + end + + it 'prints hard lines' do + doc = B.group(B.concat(["asd", B::HARD_LINE, "123"])) + expect(print(doc)).to eql("asd\n123") + expect(print(doc, print_width: 4)).to eql("asd\n123") + end + + it 'prints literal lines' do + doc = B.group(B.concat(["asd", B::LITERAL_LINE, "123"])) + expect(print(doc)).to eql("asd\n123") + expect(print(doc, print_width: 4)).to eql("asd\n123") + end + + it 'prints indents' do + doc = B.indent(B.concat([B::HARD_LINE, "abc123"])) + expect(print(doc)).to eql("\n abc123") + end + + it 'does not print indents for literal lines' do + doc = B.indent(B.concat([B::LITERAL_LINE, "abc123"])) + expect(print(doc)).to eql("\nabc123") + end + + it "prints alignments" do + doc = B.align(1, B.concat([B::HARD_LINE, "abc123"])) + expect(print(doc)).to eql("\n abc123") + end + + describe "printing of groups" do + it 'prints simple groups' do + doc = B.group(B.concat(["asd", B::SOFT_LINE, "123"])) + expect(print(doc)).to eql('asd123') + end + + it 'prints groups that should break' do + doc = B.group(B.concat(["asd", B::SOFT_LINE, "123"]), should_break: true) + expect(print(doc)).to eql("asd\n123") + end + end + + it "prints conditional groups" do + doc_states = B.conditional_group([B.concat(["asd"]), B.concat(["as"])]) + expect(print(doc_states)).to eql("asd") + expect(print(doc_states, print_width: 2)).to eql("as") + end + + it "prints fill" do + doc = B.fill(["asd", B::LINE, "123", B::LINE, "qwe"]) + expect(print(doc)).to eql("asd 123 qwe") + expect(print(doc, print_width: 8)).to eql("asd 123\nqwe") + expect(print(doc, print_width: 4)).to eql("asd\n123\nqwe") + end + + it "prints line suffixes" do + doc = B.concat(["asd", B.line_suffix("qwe"), "123", B::HARD_LINE]) + expect(print(doc)).to eql("asd123qwe\n") + end + + it "prints joins" do + doc = B.join(", ", ["asd", "123"]) + expect(print(doc)).to eql("asd, 123") + end + + it "prints cursor positions" do + doc = B.concat(["asd", B::CURSOR, "123"]) + expect(print_doc(doc)).to eql(formatted: "asd123", cursor: 3) + end +end From a794fbbc8575451e3d0059cbfc7523571b4e5e05 Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Sun, 1 Oct 2017 10:58:16 +0200 Subject: [PATCH 02/43] Fix tests for older versions of ruby --- lib/rufo/doc_printer.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/rufo/doc_printer.rb b/lib/rufo/doc_printer.rb index f3d086cf..bddfef41 100644 --- a/lib/rufo/doc_printer.rb +++ b/lib/rufo/doc_printer.rb @@ -22,7 +22,6 @@ def print_doc_to_string(doc, opts) while cmds.length != 0 x = cmds.pop - puts x.inspect ind = x[0] mode = x[1] doc = x[2] @@ -30,7 +29,6 @@ def print_doc_to_string(doc, opts) out.push(doc) pos += doc.length else - puts doc.inspect case doc[:type] when :cursor out.push(doc[:placeholder]) @@ -173,7 +171,7 @@ def print_doc_to_string(doc, opts) pos = 0 else if out.length > 0 - while out.length > 0 && out.last.match?(/^[^\S\n]*$/) + while out.length > 0 && (out.last =~ /^[^\S\n]*$/) out.pop end From d4558e1765fcee718609c3dd4c0576ccffb868cf Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Tue, 3 Oct 2017 22:47:03 +0200 Subject: [PATCH 03/43] Add more complex example and fix indent printing --- lib/rufo/doc_builder.rb | 4 +-- lib/rufo/doc_printer.rb | 2 +- spec/lib/rufo/doc_printer_spec.rb | 43 +++++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 3 deletions(-) diff --git a/lib/rufo/doc_builder.rb b/lib/rufo/doc_builder.rb index 69df8494..200dc440 100644 --- a/lib/rufo/doc_builder.rb +++ b/lib/rufo/doc_builder.rb @@ -54,8 +54,8 @@ def fill(parts) # Print first arg if the group breaks otherwise print the second. def if_break(break_contents, flat_contents) - assert_doc(break_contents) if break_contents.present? - assert_doc(flat_contents) if flat_contents.present? + assert_doc(break_contents) unless break_contents.nil? + assert_doc(flat_contents) unless flat_contents.nil? {type: :if_break, break_contents: break_contents, flat_contents: flat_contents} end diff --git a/lib/rufo/doc_printer.rb b/lib/rufo/doc_printer.rb index bddfef41..0cb89a86 100644 --- a/lib/rufo/doc_printer.rb +++ b/lib/rufo/doc_printer.rb @@ -249,7 +249,7 @@ def fits(next_cmd, rest_cmds, width, must_be_flat = false) when :concat doc[:parts].each { |part| cmds.push([ind, mode, part]) } when :indent - cmds.push(make_indent(ind), mode, doc[:contents]) + cmds.push([make_indent(ind), mode, doc[:contents]]) when :align cmds.push([make_align(ind, doc[:n]), mode, doc[:contents]]) when :group diff --git a/spec/lib/rufo/doc_printer_spec.rb b/spec/lib/rufo/doc_printer_spec.rb index 6d110243..c1f36006 100644 --- a/spec/lib/rufo/doc_printer_spec.rb +++ b/spec/lib/rufo/doc_printer_spec.rb @@ -97,4 +97,47 @@ def print_doc(doc, options = nil) doc = B.concat(["asd", B::CURSOR, "123"]) expect(print_doc(doc)).to eql(formatted: "asd123", cursor: 3) end + + context "array expression" do + let(:code_filled) { + <<~CODE.chomp("\n") + [1, 2, 3, 4, 5] + CODE + } + let(:code_broken) { + <<~CODE.chomp("\n") + [ + 1, + 2, + 3, + 4, + 5, + ] + CODE + } + + let(:doc) { + B.group( + B.concat([ + "[", + B.indent( + B.concat([ + B::SOFT_LINE, + B.join( + B.concat([",", B::LINE]), + ["1", "2", "3", "4", B.concat(["5", B.if_break(",", "")])] + ), + ]) + ), + B::SOFT_LINE, + "]", + ]) + ) + } + + it 'formats correctly' do + expect(print(doc, print_width: 10)).to eql(code_broken) + expect(print(doc, print_width: 80)).to eql(code_filled) + end + end end From ca3a59a59da7a20c1320b40cf4e29203ad41ded0 Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Sun, 12 Nov 2017 17:22:13 +0100 Subject: [PATCH 04/43] Add initial prettier printing integration --- lib/rufo/formatter.rb | 194 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 190 insertions(+), 4 deletions(-) diff --git a/lib/rufo/formatter.rb b/lib/rufo/formatter.rb index b5af2253..5d3d0e77 100644 --- a/lib/rufo/formatter.rb +++ b/lib/rufo/formatter.rb @@ -5,6 +5,7 @@ class Rufo::Formatter include Rufo::Settings + B = Rufo::DocBuilder INDENT_SIZE = 2 attr_reader :squiggly_flag @@ -398,7 +399,10 @@ def visit(node) when :params visit_params(node) when :array - visit_array(node) + puts node.inspect + doc = visit_array(node) + puts doc.inspect + @output << Rufo::DocPrinter.print_doc_to_string(doc, {print_width: 80})[:formatted] when :hash visit_hash(node) when :assoc_new @@ -2110,21 +2114,42 @@ def visit_array(node) _, elements = node + doc = [] + # doc = ["1", "2", "3", "4", B.concat(["5", B.if_break(",", "")])] token_column = current_token_column check :on_lbracket - write "[" + # write "[" next_token if elements - visit_literal_elements to_ary(elements), inside_array: true, token_column: token_column + doc = with_doc_mode { + visit_literal_elements_doc(to_ary(elements), inside_array: true, token_column: token_column) + } else skip_space_or_newline end check :on_rbracket - write "]" + # write "]" next_token + + B.group( + B.concat([ + "[", + B.indent( + B.concat([ + B::SOFT_LINE, + B.join( + B.concat([",", B::LINE]), + doc + ), + ]) + ), + B::SOFT_LINE, + "]", + ]) + ) end def visit_q_or_i_array(node) @@ -2673,6 +2698,121 @@ def visit_literal_elements(elements, inside_hash: false, inside_array: false, to end end + # def visit_literal_elements_doc(elements, inside_hash: false, inside_array: false, token_column:) + # doc = [] + # # base_column = @column + # # base_line = @line + # # needs_final_space = (inside_hash || inside_array) && space? + # # first_space = skip_space + + # # if inside_hash + # # needs_final_space = false + # # end + + # # if inside_array + # # needs_final_space = false + # # end + + # # if newline? || comment? + # # needs_final_space = false + # # end + + # # # If there's a newline right at the beginning, + # # # write it, and we'll indent element and always + # # # add a trailing comma to the last element + # # needs_trailing_comma = newline? || comment? + # # if needs_trailing_comma + # # if (call_info = @line_to_call_info[@line]) + # # call_info << true + # # end + + # # needed_indent = next_indent + # # indent { consume_end_of_line } + # # write_indent(needed_indent) + # # else + # # needed_indent = base_column + # # end + + # # wrote_comma = false + # # first_space = nil + + # elements.each_with_index do |elem, i| + # puts elem.inspect + # # comments = skip_space_or_newline_doc + # # puts "comments = #{comments}" + # doc << visit(elem) + # # comments = skip_space_or_newline_doc + # # puts "comments = #{comments}" + # next unless comma? + # next_token + # # comments = skip_space_or_newline_doc + # # puts "comments = #{comments}" + + # # We have to be careful not to aumatically write a heredoc on next_token, + # # because we miss the chance to write a comma to separate elements + # # first_space = skip_space_no_heredoc_check + # # wrote_comma = check_heredocs_in_literal_elements(is_last, needs_trailing_comma, wrote_comma) + + # # next unless comma? + + # # unless is_last + # # write "," + # # wrote_comma = true + # # end + + # # # We have to be careful not to aumatically write a heredoc on next_token, + # # # because we miss the chance to write a comma to separate elements + # # next_token_no_heredoc_check + + # # first_space = skip_space_no_heredoc_check + # # wrote_comma = check_heredocs_in_literal_elements(is_last, needs_trailing_comma, wrote_comma) + + # # if newline? || comment? + # # if is_last + # # # Nothing + # # else + # # indent(needed_indent) do + # # consume_end_of_line(first_space: first_space) + # # write_indent + # # end + # # end + # # else + # # write_space unless is_last + # # end + # end + + # # if needs_trailing_comma + # # write "," unless wrote_comma || !trailing_commas + + # # consume_end_of_line(first_space: first_space) + # # write_indent + # # elsif comment? + # # consume_end_of_line(first_space: first_space) + # # else + # # if needs_final_space + # # consume_space + # # else + # # skip_space_or_newline + # # end + # # end + + # # if current_token_column == token_column && needed_indent < token_column + # # # If the closing token is aligned with the opening token, we want to + # # # keep it like that, for example in: + # # # + # # # foo([ + # # # 2, + # # # ]) + # # @literal_indents << [base_line, @line, token_column + INDENT_SIZE - needed_indent] + # # elsif call_info && call_info[0] == current_token_column + # # # If the closing literal position matches the column where + # # # the call started, we want to preserve it like that + # # # (otherwise we align it to the first parameter) + # # call_info << @line + # # end + # doc + # end + def check_heredocs_in_literal_elements(is_last, needs_trailing_comma, wrote_comma) if (newline? || comment?) && !@heredocs.empty? if !is_last || needs_trailing_comma @@ -2974,6 +3114,52 @@ def skip_space_or_newline(_want_semicolon: false, write_first_semicolon: false) found_semicolon end + # def skip_space_or_newline_doc(_want_semicolon: false, write_first_semicolon: false) + # found_newline = false + # found_comment = false + # found_semicolon = false + # last = nil + # comments = [] + # loop do + # case current_token_kind + # when :on_sp + # next_token + # when :on_nl, :on_ignored_nl + # next_token + # last = :newline + # found_newline = true + # when :on_semicolon + # if (!found_newline && !found_comment) || (!found_semicolon && write_first_semicolon) + # write "; " + # end + # next_token + # last = :semicolon + # found_semicolon = true + # when :on_comment + # write_line if last == :newline + + # write_indent if found_comment + # if current_token_value.end_with?("\n") + # write_space + # write current_token_value.rstrip + # write "\n" + # write_indent(next_indent) + # @column = next_indent + # else + # write current_token_value + # end + # comments << current_token_value + # next_token + # found_comment = true + # last = :comment + # else + # break + # end + # end + + # comments + # end + def skip_semicolons while semicolon? || space? next_token From fbe2c4271628fc3373bfdb2a5207a8dce7c26b4a Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Sun, 12 Nov 2017 17:25:45 +0100 Subject: [PATCH 05/43] Add doc mode helpers --- lib/rufo/formatter.rb | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/lib/rufo/formatter.rb b/lib/rufo/formatter.rb index 5d3d0e77..6567f9bc 100644 --- a/lib/rufo/formatter.rb +++ b/lib/rufo/formatter.rb @@ -3179,7 +3179,7 @@ def consume_token(kind) end def consume_token_value(value) - write value + write value unless in_doc_mode? # If the value has newlines, we need to adjust line and column number_of_lines = value.count("\n") @@ -3921,4 +3921,15 @@ def remove_lines_before_inline_declarations def result @output end + + def in_doc_mode? + @in_doc_mode == true + end + + def with_doc_mode + @in_doc_mode = true + result = yield + @in_doc_mode = false + result + end end From 94ddb11acb059765e570e4b03a2eb7040a81c370 Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Sun, 12 Nov 2017 17:26:00 +0100 Subject: [PATCH 06/43] Return token from consume_token --- lib/rufo/formatter.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/rufo/formatter.rb b/lib/rufo/formatter.rb index 6567f9bc..3eefa8fa 100644 --- a/lib/rufo/formatter.rb +++ b/lib/rufo/formatter.rb @@ -3174,8 +3174,10 @@ def empty_body?(body) def consume_token(kind) check kind - consume_token_value(current_token_value) + val = current_token_value + consume_token_value(val) next_token + val end def consume_token_value(value) From 6b2895b28eb28071945f44b129f3c6a21c334fdb Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Tue, 14 Nov 2017 22:38:46 +0100 Subject: [PATCH 07/43] Allow print_width to be configured --- lib/rufo/formatter.rb | 345 ++++++++++-------- lib/rufo/settings.rb | 13 +- spec/lib/rufo/doc_printer_spec.rb | 31 ++ .../trailing_commas.rb.spec | 22 +- spec/lib/rufo/formatter_spec.rb | 3 +- 5 files changed, 246 insertions(+), 168 deletions(-) diff --git a/lib/rufo/formatter.rb b/lib/rufo/formatter.rb index 3eefa8fa..e6809b0f 100644 --- a/lib/rufo/formatter.rb +++ b/lib/rufo/formatter.rb @@ -402,7 +402,7 @@ def visit(node) puts node.inspect doc = visit_array(node) puts doc.inspect - @output << Rufo::DocPrinter.print_doc_to_string(doc, {print_width: 80})[:formatted] + @output << Rufo::DocPrinter.print_doc_to_string(doc, {print_width: print_width})[:formatted] when :hash visit_hash(node) when :assoc_new @@ -2123,7 +2123,7 @@ def visit_array(node) next_token if elements - doc = with_doc_mode { + doc, pre_comments, post_comments, has_comment = with_doc_mode { visit_literal_elements_doc(to_ary(elements), inside_array: true, token_column: token_column) } else @@ -2133,12 +2133,18 @@ def visit_array(node) check :on_rbracket # write "]" next_token + puts trailing_commas.inspect + if trailing_commas + last = doc.pop + doc << B.concat([last, B.if_break(",", "")]) + end B.group( B.concat([ "[", B.indent( B.concat([ + B.concat(pre_comments), B::SOFT_LINE, B.join( B.concat([",", B::LINE]), @@ -2148,7 +2154,8 @@ def visit_array(node) ), B::SOFT_LINE, "]", - ]) + ]), + should_break: has_comment ) end @@ -2698,120 +2705,136 @@ def visit_literal_elements(elements, inside_hash: false, inside_array: false, to end end - # def visit_literal_elements_doc(elements, inside_hash: false, inside_array: false, token_column:) - # doc = [] - # # base_column = @column - # # base_line = @line - # # needs_final_space = (inside_hash || inside_array) && space? - # # first_space = skip_space - - # # if inside_hash - # # needs_final_space = false - # # end - - # # if inside_array - # # needs_final_space = false - # # end - - # # if newline? || comment? - # # needs_final_space = false - # # end - - # # # If there's a newline right at the beginning, - # # # write it, and we'll indent element and always - # # # add a trailing comma to the last element - # # needs_trailing_comma = newline? || comment? - # # if needs_trailing_comma - # # if (call_info = @line_to_call_info[@line]) - # # call_info << true - # # end - - # # needed_indent = next_indent - # # indent { consume_end_of_line } - # # write_indent(needed_indent) - # # else - # # needed_indent = base_column - # # end - - # # wrote_comma = false - # # first_space = nil - - # elements.each_with_index do |elem, i| - # puts elem.inspect - # # comments = skip_space_or_newline_doc - # # puts "comments = #{comments}" - # doc << visit(elem) - # # comments = skip_space_or_newline_doc - # # puts "comments = #{comments}" - # next unless comma? - # next_token - # # comments = skip_space_or_newline_doc - # # puts "comments = #{comments}" - - # # We have to be careful not to aumatically write a heredoc on next_token, - # # because we miss the chance to write a comma to separate elements - # # first_space = skip_space_no_heredoc_check - # # wrote_comma = check_heredocs_in_literal_elements(is_last, needs_trailing_comma, wrote_comma) - - # # next unless comma? - - # # unless is_last - # # write "," - # # wrote_comma = true - # # end - - # # # We have to be careful not to aumatically write a heredoc on next_token, - # # # because we miss the chance to write a comma to separate elements - # # next_token_no_heredoc_check - - # # first_space = skip_space_no_heredoc_check - # # wrote_comma = check_heredocs_in_literal_elements(is_last, needs_trailing_comma, wrote_comma) - - # # if newline? || comment? - # # if is_last - # # # Nothing - # # else - # # indent(needed_indent) do - # # consume_end_of_line(first_space: first_space) - # # write_indent - # # end - # # end - # # else - # # write_space unless is_last - # # end - # end - - # # if needs_trailing_comma - # # write "," unless wrote_comma || !trailing_commas - - # # consume_end_of_line(first_space: first_space) - # # write_indent - # # elsif comment? - # # consume_end_of_line(first_space: first_space) - # # else - # # if needs_final_space - # # consume_space - # # else - # # skip_space_or_newline - # # end - # # end - - # # if current_token_column == token_column && needed_indent < token_column - # # # If the closing token is aligned with the opening token, we want to - # # # keep it like that, for example in: - # # # - # # # foo([ - # # # 2, - # # # ]) - # # @literal_indents << [base_line, @line, token_column + INDENT_SIZE - needed_indent] - # # elsif call_info && call_info[0] == current_token_column - # # # If the closing literal position matches the column where - # # # the call started, we want to preserve it like that - # # # (otherwise we align it to the first parameter) - # # call_info << @line - # # end - # doc - # end + def add_comments_to_doc(comments, doc) + return false if comments.empty? + + comments.each do |c| + doc << B.concat([B.line_suffix(" " + c.rstrip), B::LINE_SUFFIX_BOUNDARY]) + end + return true + end + + def visit_literal_elements_doc(elements, inside_hash: false, inside_array: false, token_column:) + doc = [] + pre_comments = [] + post_comments = [] + has_comment = false + # base_column = @column + # base_line = @line + # needs_final_space = (inside_hash || inside_array) && space? + # first_space = skip_space + + # if inside_hash + # needs_final_space = false + # end + + # if inside_array + # needs_final_space = false + # end + + # if newline? || comment? + # needs_final_space = false + # end + + # # If there's a newline right at the beginning, + # # write it, and we'll indent element and always + # # add a trailing comma to the last element + # needs_trailing_comma = newline? || comment? + # if needs_trailing_comma + # if (call_info = @line_to_call_info[@line]) + # call_info << true + # end + + # needed_indent = next_indent + # indent { consume_end_of_line } + # write_indent(needed_indent) + # else + # needed_indent = base_column + # end + + # wrote_comma = false + # first_space = nil + + comments = skip_space_or_newline_doc + puts "comments 1 = #{comments}" + has_comment ||= add_comments_to_doc(comments, pre_comments) + elements.each_with_index do |elem, i| + puts elem.inspect + doc << visit(elem) + comments = skip_space_or_newline_doc + puts "comments 2 = #{comments}" + has_comment ||= add_comments_to_doc(comments, doc) + + next unless comma? + next_token + comments = skip_space_or_newline_doc + puts "comments 3 = #{comments}" + has_comment ||= add_comments_to_doc(comments, doc) + + # We have to be careful not to aumatically write a heredoc on next_token, + # because we miss the chance to write a comma to separate elements + # first_space = skip_space_no_heredoc_check + # wrote_comma = check_heredocs_in_literal_elements(is_last, needs_trailing_comma, wrote_comma) + + # next unless comma? + + # unless is_last + # write "," + # wrote_comma = true + # end + + # # We have to be careful not to aumatically write a heredoc on next_token, + # # because we miss the chance to write a comma to separate elements + # next_token_no_heredoc_check + + # first_space = skip_space_no_heredoc_check + # wrote_comma = check_heredocs_in_literal_elements(is_last, needs_trailing_comma, wrote_comma) + + # if newline? || comment? + # if is_last + # # Nothing + # else + # indent(needed_indent) do + # consume_end_of_line(first_space: first_space) + # write_indent + # end + # end + # else + # write_space unless is_last + # end + end + + # if needs_trailing_comma + # write "," unless wrote_comma || !trailing_commas + + # consume_end_of_line(first_space: first_space) + # write_indent + # elsif comment? + # consume_end_of_line(first_space: first_space) + # else + # if needs_final_space + # consume_space + # else + # skip_space_or_newline + # end + # end + + # if current_token_column == token_column && needed_indent < token_column + # # If the closing token is aligned with the opening token, we want to + # # keep it like that, for example in: + # # + # # foo([ + # # 2, + # # ]) + # @literal_indents << [base_line, @line, token_column + INDENT_SIZE - needed_indent] + # elsif call_info && call_info[0] == current_token_column + # # If the closing literal position matches the column where + # # the call started, we want to preserve it like that + # # (otherwise we align it to the first parameter) + # call_info << @line + # end + [doc, pre_comments, post_comments, has_comment] + end def check_heredocs_in_literal_elements(is_last, needs_trailing_comma, wrote_comma) if (newline? || comment?) && !@heredocs.empty? @@ -3114,51 +3137,51 @@ def skip_space_or_newline(_want_semicolon: false, write_first_semicolon: false) found_semicolon end - # def skip_space_or_newline_doc(_want_semicolon: false, write_first_semicolon: false) - # found_newline = false - # found_comment = false - # found_semicolon = false - # last = nil - # comments = [] - # loop do - # case current_token_kind - # when :on_sp - # next_token - # when :on_nl, :on_ignored_nl - # next_token - # last = :newline - # found_newline = true - # when :on_semicolon - # if (!found_newline && !found_comment) || (!found_semicolon && write_first_semicolon) - # write "; " - # end - # next_token - # last = :semicolon - # found_semicolon = true - # when :on_comment - # write_line if last == :newline - - # write_indent if found_comment - # if current_token_value.end_with?("\n") - # write_space - # write current_token_value.rstrip - # write "\n" - # write_indent(next_indent) - # @column = next_indent - # else - # write current_token_value - # end - # comments << current_token_value - # next_token - # found_comment = true - # last = :comment - # else - # break - # end - # end + def skip_space_or_newline_doc(_want_semicolon: false, write_first_semicolon: false) + found_newline = false + found_comment = false + found_semicolon = false + last = nil + comments = [] + loop do + case current_token_kind + when :on_sp + next_token + when :on_nl, :on_ignored_nl + next_token + last = :newline + found_newline = true + when :on_semicolon + if (!found_newline && !found_comment) || (!found_semicolon && write_first_semicolon) + # write "; " + end + next_token + last = :semicolon + found_semicolon = true + when :on_comment + # write_line if last == :newline - # comments - # end + # write_indent if found_comment + if current_token_value.end_with?("\n") + # write_space + # write current_token_value.rstrip + # write "\n" + # write_indent(next_indent) + @column = next_indent + else + # write current_token_value + end + comments << current_token_value + next_token + found_comment = true + last = :comment + else + break + end + end + + comments + end def skip_semicolons while semicolon? || space? diff --git a/lib/rufo/settings.rb b/lib/rufo/settings.rb index cad4236a..c66971fc 100644 --- a/lib/rufo/settings.rb +++ b/lib/rufo/settings.rb @@ -4,17 +4,26 @@ module Rufo::Settings align_case_when: [false, true], align_chained_calls: [false, true], trailing_commas: [true, false], + print_width: (1..Float::INFINITY) + } + + DEFAULT_VALUES = { + print_width: 80 } attr_accessor *OPTIONS.keys def init_settings(options) OPTIONS.each do |name, valid_options| - default = valid_options.first + if DEFAULT_VALUES.has_key?(name) + default = DEFAULT_VALUES[name] + else + default = valid_options.first + end value = options.fetch(name, default) unless valid_options.include?(value) $stderr.puts "Invalid value for #{name}: #{value.inspect}. Valid " \ - "values are: #{valid_options.map(&:inspect).join(', ')}" + "values are: #{valid_options.map(&:inspect).join(', ')}" value = default end self.public_send("#{name}=", value) diff --git a/spec/lib/rufo/doc_printer_spec.rb b/spec/lib/rufo/doc_printer_spec.rb index c1f36006..83407fb3 100644 --- a/spec/lib/rufo/doc_printer_spec.rb +++ b/spec/lib/rufo/doc_printer_spec.rb @@ -140,4 +140,35 @@ def print_doc(doc, options = nil) expect(print(doc, print_width: 80)).to eql(code_filled) end end + + context 'array with comment' do + let(:doc) { + B.group( + B.concat([ + "[", + B.indent( + B.concat([ + B::SOFT_LINE, + B.join( + B.concat([",", B::LINE]), + [B.concat(["1", B.line_suffix(' # a comment'), B::LINE_SUFFIX_BOUNDARY])] + ), + ]) + ), + B::SOFT_LINE, + "]", + ]), + should_break: true + ) + } + + it 'formats array with comment' do + expect(print(doc, print_width: 80)).to eql(<<~CODE.chomp("\n") + [ + 1 # a comment + ] + CODE + ) + end + end end diff --git a/spec/lib/rufo/formatter_source_specs/trailing_commas.rb.spec b/spec/lib/rufo/formatter_source_specs/trailing_commas.rb.spec index 92489682..56b25086 100644 --- a/spec/lib/rufo/formatter_source_specs/trailing_commas.rb.spec +++ b/spec/lib/rufo/formatter_source_specs/trailing_commas.rb.spec @@ -1,5 +1,6 @@ #~# ORIGINAL #~# trailing_commas: true +#~# print_width: 5 [ 1, @@ -15,6 +16,7 @@ #~# ORIGINAL #~# trailing_commas: false +#~# print_width: 5 [ 1, @@ -30,6 +32,7 @@ #~# ORIGINAL #~# trailing_commas: true +#~# print_width: 5 [ 1, @@ -45,6 +48,7 @@ #~# ORIGINAL #~# trailing_commas: false +#~# print_width: 5 [ 1, @@ -236,6 +240,7 @@ foo( #~# ORIGINAL #~# trailing_commas: true +#~# print_width: 5 [ 1 , 2 ] @@ -243,11 +248,13 @@ foo( #~# EXPECTED [ - 1, 2, + 1, + 2, ] #~# ORIGINAL #~# trailing_commas: true +#~# print_width: 5 [ 1 , 2, ] @@ -255,11 +262,13 @@ foo( #~# EXPECTED [ - 1, 2, + 1, + 2, ] #~# ORIGINAL #~# trailing_commas: true +#~# print_width: 5 [ 1 , 2 , @@ -268,12 +277,15 @@ foo( #~# EXPECTED [ - 1, 2, - 3, 4, + 1, + 2, + 3, + 4, ] #~# ORIGINAL #~# trailing_commas: true +#~# print_width: 5 [ 1 , @@ -316,6 +328,7 @@ foo( #~# ORIGINAL #~# trailing_commas: true +#~# print_width: 6 [ 1 , 2, 3, @@ -371,6 +384,7 @@ foo( #~# ORIGINAL #~# trailing_commas: true +#~# print_width: 5 begin [ diff --git a/spec/lib/rufo/formatter_spec.rb b/spec/lib/rufo/formatter_spec.rb index 6ceb329b..cea3ba16 100644 --- a/spec/lib/rufo/formatter_spec.rb +++ b/spec/lib/rufo/formatter_spec.rb @@ -29,7 +29,8 @@ def assert_source_specs(source_specs) when line =~ /^#~# PENDING$/ current_test[:pending] = true when line =~ /^#~# (.+)$/ - current_test[:options] = eval("{ #{$~[1]} }") + current_options = current_test[:options] || {} + current_test[:options] = current_options.merge(eval("{ #{$~[1]} }")) when current_test[:expected] current_test[:expected] += line when current_test[:original] From 656636fc501a4004b6716e2d71ffd2eb5a545d7a Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Tue, 14 Nov 2017 22:44:08 +0100 Subject: [PATCH 08/43] Fix more tests Why: --- lib/rufo/formatter.rb | 4 +-- .../trailing_commas.rb.spec | 34 +++++++++++++------ 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/lib/rufo/formatter.rb b/lib/rufo/formatter.rb index e6809b0f..c97c24b0 100644 --- a/lib/rufo/formatter.rb +++ b/lib/rufo/formatter.rb @@ -400,9 +400,9 @@ def visit(node) visit_params(node) when :array puts node.inspect - doc = visit_array(node) + doc = B.align(@indent, visit_array(node)) puts doc.inspect - @output << Rufo::DocPrinter.print_doc_to_string(doc, {print_width: print_width})[:formatted] + @output << Rufo::DocPrinter.print_doc_to_string(doc, {print_width: print_width - @indent})[:formatted] when :hash visit_hash(node) when :assoc_new diff --git a/spec/lib/rufo/formatter_source_specs/trailing_commas.rb.spec b/spec/lib/rufo/formatter_source_specs/trailing_commas.rb.spec index 56b25086..6a3dc871 100644 --- a/spec/lib/rufo/formatter_source_specs/trailing_commas.rb.spec +++ b/spec/lib/rufo/formatter_source_specs/trailing_commas.rb.spec @@ -328,7 +328,7 @@ foo( #~# ORIGINAL #~# trailing_commas: true -#~# print_width: 6 +#~# print_width: 5 [ 1 , 2, 3, @@ -336,12 +336,16 @@ foo( #~# EXPECTED -[1, - 2, 3, - 4] +[ + 1, + 2, + 3, + 4, +] #~# ORIGINAL #~# trailing_commas: true +#~# print_width: 5 [ 1 , 2, 3, @@ -349,12 +353,16 @@ foo( #~# EXPECTED -[1, - 2, 3, - 4] +[ + 1, + 2, + 3, + 4, +] #~# ORIGINAL #~# trailing_commas: true +#~# print_width: 5 [ 1 , 2, 3, @@ -363,9 +371,12 @@ foo( #~# EXPECTED -[1, - 2, 3, - 4] +[ + 1, + 2, + 3, + 4, +] #~# ORIGINAL #~# trailing_commas: true @@ -395,7 +406,8 @@ foo( begin [ - 1, 2, + 1, + 2, ] end From 85e61650fdd544927c5b35bceaa5dffaa8d55070 Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Tue, 14 Nov 2017 22:54:27 +0100 Subject: [PATCH 09/43] Fix test for comment on first element Why: --- lib/rufo/formatter.rb | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/lib/rufo/formatter.rb b/lib/rufo/formatter.rb index c97c24b0..d71dd0c1 100644 --- a/lib/rufo/formatter.rb +++ b/lib/rufo/formatter.rb @@ -2147,7 +2147,7 @@ def visit_array(node) B.concat(pre_comments), B::SOFT_LINE, B.join( - B.concat([",", B::LINE]), + B.concat([",", B::LINE_SUFFIX_BOUNDARY, B::LINE]), doc ), ]) @@ -2709,7 +2709,7 @@ def add_comments_to_doc(comments, doc) return false if comments.empty? comments.each do |c| - doc << B.concat([B.line_suffix(" " + c.rstrip), B::LINE_SUFFIX_BOUNDARY]) + doc << B.line_suffix(" " + c.rstrip) end return true end @@ -2763,7 +2763,12 @@ def visit_literal_elements_doc(elements, inside_hash: false, inside_array: false doc << visit(elem) comments = skip_space_or_newline_doc puts "comments 2 = #{comments}" - has_comment ||= add_comments_to_doc(comments, doc) + unless comments.empty? + has_comment = true + first_comment = comments.shift + doc << B.concat([doc.pop, B.line_suffix(" " + first_comment.rstrip)]) + end + add_comments_to_doc(comments, doc) next unless comma? next_token From 8867e8b0903c2eea71c43e37b158c124dcd3a839 Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Tue, 14 Nov 2017 22:59:58 +0100 Subject: [PATCH 10/43] Fix remaining trailing comma tests Why: --- lib/rufo/formatter.rb | 7 ++++++- .../formatter_source_specs/trailing_commas.rb.spec | 10 ++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/lib/rufo/formatter.rb b/lib/rufo/formatter.rb index d71dd0c1..738e0af4 100644 --- a/lib/rufo/formatter.rb +++ b/lib/rufo/formatter.rb @@ -2774,7 +2774,12 @@ def visit_literal_elements_doc(elements, inside_hash: false, inside_array: false next_token comments = skip_space_or_newline_doc puts "comments 3 = #{comments}" - has_comment ||= add_comments_to_doc(comments, doc) + unless comments.empty? + has_comment = true + first_comment = comments.shift + doc << B.concat([doc.pop, B.line_suffix(" " + first_comment.rstrip)]) + end + add_comments_to_doc(comments, doc) # We have to be careful not to aumatically write a heredoc on next_token, # because we miss the chance to write a comma to separate elements diff --git a/spec/lib/rufo/formatter_source_specs/trailing_commas.rb.spec b/spec/lib/rufo/formatter_source_specs/trailing_commas.rb.spec index 6a3dc871..b60543d9 100644 --- a/spec/lib/rufo/formatter_source_specs/trailing_commas.rb.spec +++ b/spec/lib/rufo/formatter_source_specs/trailing_commas.rb.spec @@ -322,7 +322,7 @@ foo( #~# EXPECTED [ - 1, # comment + 1, # comment 2, ] @@ -388,9 +388,11 @@ foo( #~# EXPECTED -[1, - 2, 3, - 4 # foo +[ + 1, + 2, + 3, + 4, # foo ] #~# ORIGINAL From fdd624287be94a15219d698d0707bf696eaf540e Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Tue, 14 Nov 2017 23:14:44 +0100 Subject: [PATCH 11/43] Fix simple method call spec --- lib/rufo/formatter.rb | 2 +- .../method_calls.rb.spec | 26 ++++++++++++------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/lib/rufo/formatter.rb b/lib/rufo/formatter.rb index 738e0af4..850ffedd 100644 --- a/lib/rufo/formatter.rb +++ b/lib/rufo/formatter.rb @@ -402,7 +402,7 @@ def visit(node) puts node.inspect doc = B.align(@indent, visit_array(node)) puts doc.inspect - @output << Rufo::DocPrinter.print_doc_to_string(doc, {print_width: print_width - @indent})[:formatted] + @output << Rufo::DocPrinter.print_doc_to_string(doc, {print_width: print_width - @indent - (@column - @indent)})[:formatted] when :hash visit_hash(node) when :assoc_new diff --git a/spec/lib/rufo/formatter_source_specs/method_calls.rb.spec b/spec/lib/rufo/formatter_source_specs/method_calls.rb.spec index ccc5a16e..d7d34de2 100644 --- a/spec/lib/rufo/formatter_source_specs/method_calls.rb.spec +++ b/spec/lib/rufo/formatter_source_specs/method_calls.rb.spec @@ -490,6 +490,7 @@ foo 1, { } #~# ORIGINAL +#~# print_width: 7 foo 1, [ 1, @@ -498,8 +499,8 @@ foo 1, [ #~# EXPECTED foo 1, [ - 1, -] + 1, + ] #~# ORIGINAL @@ -584,6 +585,7 @@ foo(& block) foo(&block) #~# ORIGINAL +#~# print_width: 7 foo 1, [ 2, @@ -596,6 +598,7 @@ foo 1, [ ] #~# ORIGINAL +#~# print_width: 7 foo 1, [ 2, @@ -604,8 +607,8 @@ foo 1, [ #~# EXPECTED foo 1, [ - 2, -] + 2, + ] #~# ORIGINAL @@ -680,6 +683,7 @@ begin end #~# ORIGINAL +#~# print_width: 6 foo([ 1, @@ -688,10 +692,11 @@ foo([ #~# EXPECTED foo([ - 1, - ]) + 1, +]) #~# ORIGINAL +#~# print_width: 7 begin foo([ @@ -703,11 +708,12 @@ end begin foo([ - 1, - ]) + 1, + ]) end #~# ORIGINAL +#~# print_width: 9 (a b).c([ 1, @@ -716,8 +722,8 @@ end #~# EXPECTED (a b).c([ - 1, - ]) + 1, +]) #~# ORIGINAL From a93cd49e38707ffea2c6d42ee926bec6c1702b5c Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Tue, 14 Nov 2017 23:18:12 +0100 Subject: [PATCH 12/43] Fix percent array literal test --- lib/rufo/formatter.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/rufo/formatter.rb b/lib/rufo/formatter.rb index 850ffedd..871bc171 100644 --- a/lib/rufo/formatter.rb +++ b/lib/rufo/formatter.rb @@ -400,7 +400,9 @@ def visit(node) visit_params(node) when :array puts node.inspect - doc = B.align(@indent, visit_array(node)) + doc = visit_array(node) + return if doc.nil? + doc = B.align(@indent, doc) puts doc.inspect @output << Rufo::DocPrinter.print_doc_to_string(doc, {print_width: print_width - @indent - (@column - @indent)})[:formatted] when :hash From 903e72a128f5f6d9ef31590b76600a8b540f8872 Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Wed, 15 Nov 2017 17:38:26 +0100 Subject: [PATCH 13/43] Fix simple array specs --- lib/rufo/formatter.rb | 6 ++- .../array_literal.rb.spec | 48 +++++++++++-------- 2 files changed, 32 insertions(+), 22 deletions(-) diff --git a/lib/rufo/formatter.rb b/lib/rufo/formatter.rb index 871bc171..bea38985 100644 --- a/lib/rufo/formatter.rb +++ b/lib/rufo/formatter.rb @@ -2117,7 +2117,9 @@ def visit_array(node) _, elements = node doc = [] - # doc = ["1", "2", "3", "4", B.concat(["5", B.if_break(",", "")])] + pre_comments = [] + post_comments = [] + has_comment = false token_column = current_token_column check :on_lbracket @@ -2136,7 +2138,7 @@ def visit_array(node) # write "]" next_token puts trailing_commas.inspect - if trailing_commas + if trailing_commas && !doc.empty? last = doc.pop doc << B.concat([last, B.if_break(",", "")]) end diff --git a/spec/lib/rufo/formatter_source_specs/array_literal.rb.spec b/spec/lib/rufo/formatter_source_specs/array_literal.rb.spec index 25f04a80..9c12821a 100644 --- a/spec/lib/rufo/formatter_source_specs/array_literal.rb.spec +++ b/spec/lib/rufo/formatter_source_specs/array_literal.rb.spec @@ -31,6 +31,7 @@ [1, 2] #~# ORIGINAL +#~# print_width: 4 [ 1 , 2 ] @@ -38,10 +39,12 @@ #~# EXPECTED [ - 1, 2, + 1, + 2, ] #~# ORIGINAL +#~# print_width: 4 [ 1 , 2, ] @@ -49,10 +52,12 @@ #~# EXPECTED [ - 1, 2, + 1, + 2, ] #~# ORIGINAL +#~# print_width: 4 [ 1 , 2 , @@ -61,11 +66,14 @@ #~# EXPECTED [ - 1, 2, - 3, 4, + 1, + 2, + 3, + 4, ] #~# ORIGINAL +#~# print_width: 4 [ 1 , @@ -100,11 +108,12 @@ #~# EXPECTED [ - 1, # comment + 1, # comment 2, ] #~# ORIGINAL +#~# print_width: 4 [ 1 , 2, 3, @@ -112,9 +121,12 @@ #~# EXPECTED -[1, - 2, 3, - 4] +[ + 1, + 2, + 3, + 4, +] #~# ORIGINAL @@ -124,9 +136,7 @@ #~# EXPECTED -[1, - 2, 3, - 4] +[1, 2, 3, 4] #~# ORIGINAL @@ -137,9 +147,7 @@ #~# EXPECTED -[1, - 2, 3, - 4] +[1, 2, 3, 4] #~# ORIGINAL @@ -150,9 +158,11 @@ #~# EXPECTED -[1, - 2, 3, - 4 # foo +[ + 1, + 2, + 3, + 4, # foo ] #~# ORIGINAL @@ -165,9 +175,7 @@ #~# EXPECTED begin - [ - 1, 2, - ] + [1, 2] end #~# ORIGINAL From df24550e9f42dc179c8da4e7d630ac6905c78b03 Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Wed, 15 Nov 2017 18:05:37 +0100 Subject: [PATCH 14/43] Add support for star args --- lib/rufo/formatter.rb | 49 ++++++++++++++++++++++++++++++------------- 1 file changed, 35 insertions(+), 14 deletions(-) diff --git a/lib/rufo/formatter.rb b/lib/rufo/formatter.rb index bea38985..2dc0ba20 100644 --- a/lib/rufo/formatter.rb +++ b/lib/rufo/formatter.rb @@ -1438,29 +1438,44 @@ def visit_call_args(node) end end + def skip_comma_and_spaces + skip_space + check :on_comma + next_token + skip_space + end + def visit_args_add_star(node) # [:args_add_star, args, star, post_args] _, args, star, *post_args = node - + doc = [] if !args.empty? && args[0] == :args_add_star # arg1, ..., *star - visit args + doc = visit args else - visit_comma_separated_list args + pre_doc, pre_comments, post_comments, has_comment = with_doc_mode { + visit_literal_elements_doc(to_ary(args)) + } + doc.concat(pre_doc) end - skip_space + # skip_space - write_params_comma if comma? + skip_comma_and_spaces if comma? consume_op "*" - skip_space_or_newline - visit star + # skip_space_or_newline_doc + doc << "*#{visit star}" if post_args && !post_args.empty? - write_params_comma - visit_comma_separated_list post_args + skip_comma_and_spaces + # visit_comma_separated_list post_args + post_doc, pre_comments, post_comments, has_comment = with_doc_mode { + visit_literal_elements_doc(to_ary(post_args)) + } + doc.concat(post_doc) end + doc end def visit_begin(node) @@ -2718,7 +2733,7 @@ def add_comments_to_doc(comments, doc) return true end - def visit_literal_elements_doc(elements, inside_hash: false, inside_array: false, token_column:) + def visit_literal_elements_doc(elements, inside_hash: false, inside_array: false, token_column: false) doc = [] pre_comments = [] post_comments = [] @@ -2764,7 +2779,12 @@ def visit_literal_elements_doc(elements, inside_hash: false, inside_array: false has_comment ||= add_comments_to_doc(comments, pre_comments) elements.each_with_index do |elem, i| puts elem.inspect - doc << visit(elem) + doc_el = visit(elem) + if doc_el.is_a?(Array) + doc.concat(doc_el) + else + doc << doc_el + end comments = skip_space_or_newline_doc puts "comments 2 = #{comments}" unless comments.empty? @@ -3244,7 +3264,7 @@ def consume_op(value) if current_token_value != value bug "Expected op #{value}, not #{current_token_value}" end - write value + write value unless in_doc_mode? next_token end @@ -3564,7 +3584,7 @@ def maybe_indent(toggle, indent_size) end def write(value) - @output << value + @output << value unless in_doc_mode? @last_was_newline = false @last_was_heredoc = false @column += value.size @@ -3966,9 +3986,10 @@ def in_doc_mode? end def with_doc_mode + old_val = @in_doc_mode @in_doc_mode = true result = yield - @in_doc_mode = false + @in_doc_mode = old_val result end end From 6c90829724abf863a55159b4179d96fa12376fd7 Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Wed, 15 Nov 2017 18:15:27 +0100 Subject: [PATCH 15/43] Fix more complex tests --- spec/lib/rufo/formatter_source_specs/array_literal.rb.spec | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/spec/lib/rufo/formatter_source_specs/array_literal.rb.spec b/spec/lib/rufo/formatter_source_specs/array_literal.rb.spec index 9c12821a..bb43a0c1 100644 --- a/spec/lib/rufo/formatter_source_specs/array_literal.rb.spec +++ b/spec/lib/rufo/formatter_source_specs/array_literal.rb.spec @@ -251,6 +251,7 @@ x = [{ ] #~# ORIGINAL +#~# print_width: 5 [ *a, @@ -265,6 +266,7 @@ x = [{ ] #~# ORIGINAL +#~# print_width: 5 [ 1, *a, @@ -274,6 +276,7 @@ x = [{ #~# EXPECTED [ - 1, *a, + 1, + *a, b, ] From 46cc5fde55b8975dd3bb05fc27a6538d93726156 Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Wed, 15 Nov 2017 18:15:46 +0100 Subject: [PATCH 16/43] Fix hash test --- lib/rufo/formatter.rb | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/lib/rufo/formatter.rb b/lib/rufo/formatter.rb index 2dc0ba20..c7c071be 100644 --- a/lib/rufo/formatter.rb +++ b/lib/rufo/formatter.rb @@ -406,7 +406,14 @@ def visit(node) puts doc.inspect @output << Rufo::DocPrinter.print_doc_to_string(doc, {print_width: print_width - @indent - (@column - @indent)})[:formatted] when :hash - visit_hash(node) + if in_doc_mode? + capture_output { + visit_hash(node) + } + else + visit_hash(node) + end + when :assoc_new visit_hash_key_value(node) when :assoc_splat @@ -3981,6 +3988,23 @@ def result @output end + def capture_output + old_doc_mode = @in_doc_mode + @in_doc_mode = false + + old_output = @output + @output = ''.dup + + yield + + result = @output + @output = old_output + + @in_doc_mode = old_doc_mode + + result + end + def in_doc_mode? @in_doc_mode == true end From 8b015d6f3c46e33d51d3bb96dbc92b7757032106 Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Wed, 15 Nov 2017 18:20:06 +0100 Subject: [PATCH 17/43] Only use new args_add_star logic in doc mode Why: So that other usages don't need to be fixed yet. --- lib/rufo/formatter.rb | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/lib/rufo/formatter.rb b/lib/rufo/formatter.rb index c7c071be..64250e4f 100644 --- a/lib/rufo/formatter.rb +++ b/lib/rufo/formatter.rb @@ -1445,6 +1445,32 @@ def visit_call_args(node) end end + def visit_args_add_star(node) + return visit_args_add_star_doc(node) if in_doc_mode? + # [:args_add_star, args, star, post_args] + _, args, star, *post_args = node + + if !args.empty? && args[0] == :args_add_star + # arg1, ..., *star + visit args + else + visit_comma_separated_list args + end + + skip_space + + write_params_comma if comma? + + consume_op "*" + skip_space_or_newline + visit star + + if post_args && !post_args.empty? + write_params_comma + visit_comma_separated_list post_args + end + end + def skip_comma_and_spaces skip_space check :on_comma @@ -1452,7 +1478,7 @@ def skip_comma_and_spaces skip_space end - def visit_args_add_star(node) + def visit_args_add_star_doc(node) # [:args_add_star, args, star, post_args] _, args, star, *post_args = node doc = [] From 130bc9603185484fe2753150ad73ba32fa7d222e Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Wed, 15 Nov 2017 18:34:42 +0100 Subject: [PATCH 18/43] Fix comment on a newline after an array item --- lib/rufo/formatter.rb | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/lib/rufo/formatter.rb b/lib/rufo/formatter.rb index 64250e4f..7167f5d2 100644 --- a/lib/rufo/formatter.rb +++ b/lib/rufo/formatter.rb @@ -2807,7 +2807,7 @@ def visit_literal_elements_doc(elements, inside_hash: false, inside_array: false # wrote_comma = false # first_space = nil - comments = skip_space_or_newline_doc + comments, newline_before_comment = skip_space_or_newline_doc puts "comments 1 = #{comments}" has_comment ||= add_comments_to_doc(comments, pre_comments) elements.each_with_index do |elem, i| @@ -2818,7 +2818,7 @@ def visit_literal_elements_doc(elements, inside_hash: false, inside_array: false else doc << doc_el end - comments = skip_space_or_newline_doc + comments, newline_before_comment = skip_space_or_newline_doc puts "comments 2 = #{comments}" unless comments.empty? has_comment = true @@ -2829,12 +2829,18 @@ def visit_literal_elements_doc(elements, inside_hash: false, inside_array: false next unless comma? next_token - comments = skip_space_or_newline_doc + comments, newline_before_comment = skip_space_or_newline_doc puts "comments 3 = #{comments}" + puts newline_before_comment unless comments.empty? has_comment = true first_comment = comments.shift - doc << B.concat([doc.pop, B.line_suffix(" " + first_comment.rstrip)]) + + if newline_before_comment + doc << B.concat([doc.pop, B.line_suffix(B.concat([B::LINE, first_comment.rstrip]))]) + else + doc << B.concat([doc.pop, B.line_suffix(" " + first_comment.rstrip)]) + end end add_comments_to_doc(comments, doc) @@ -3208,6 +3214,7 @@ def skip_space_or_newline_doc(_want_semicolon: false, write_first_semicolon: fal found_newline = false found_comment = false found_semicolon = false + newline_before_comment = false last = nil comments = [] loop do @@ -3218,6 +3225,9 @@ def skip_space_or_newline_doc(_want_semicolon: false, write_first_semicolon: fal next_token last = :newline found_newline = true + if comments.empty? + newline_before_comment = true + end when :on_semicolon if (!found_newline && !found_comment) || (!found_semicolon && write_first_semicolon) # write "; " @@ -3247,7 +3257,7 @@ def skip_space_or_newline_doc(_want_semicolon: false, write_first_semicolon: fal end end - comments + [comments, newline_before_comment] end def skip_semicolons From 4e52126d3c711b1f91b68b1a300334b94bf7afbb Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Wed, 15 Nov 2017 18:40:24 +0100 Subject: [PATCH 19/43] Handle strings in doc mode --- lib/rufo/formatter.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/rufo/formatter.rb b/lib/rufo/formatter.rb index 7167f5d2..70933e87 100644 --- a/lib/rufo/formatter.rb +++ b/lib/rufo/formatter.rb @@ -226,7 +226,12 @@ def visit(node) # [:@backtick, "`", [1, 4]] consume_token :on_backtick when :string_literal, :xstring_literal - visit_string_literal node + if in_doc_mode? + capture_output { visit_string_literal node } + else + visit_string_literal node + end + when :string_concat visit_string_concat node when :@tstring_content From befd18d1f0841172ba335436e8f15f12c7fefbc0 Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Thu, 16 Nov 2017 09:57:59 +0100 Subject: [PATCH 20/43] Change specs to match new formatting --- .../formatter_source_specs/align_comments.rb.spec | 8 ++++---- .../formatter_source_specs/align_hash_keys.rb.spec | 12 +++--------- .../calls_with_receiver.rb.spec | 12 +++++------- 3 files changed, 12 insertions(+), 20 deletions(-) diff --git a/spec/lib/rufo/formatter_source_specs/align_comments.rb.spec b/spec/lib/rufo/formatter_source_specs/align_comments.rb.spec index 116ff455..dd5b46b3 100644 --- a/spec/lib/rufo/formatter_source_specs/align_comments.rb.spec +++ b/spec/lib/rufo/formatter_source_specs/align_comments.rb.spec @@ -168,8 +168,8 @@ bar = 2 # baz #~# EXPECTED [ - 1, # foo - 234, # bar + 1, # foo + 234, # bar ] #~# ORIGINAL @@ -182,8 +182,8 @@ bar = 2 # baz #~# EXPECTED [ - 1, # foo - 234, # bar + 1, # foo + 234, # bar ] #~# ORIGINAL diff --git a/spec/lib/rufo/formatter_source_specs/align_hash_keys.rb.spec b/spec/lib/rufo/formatter_source_specs/align_hash_keys.rb.spec index c712cbdb..450d949e 100644 --- a/spec/lib/rufo/formatter_source_specs/align_hash_keys.rb.spec +++ b/spec/lib/rufo/formatter_source_specs/align_hash_keys.rb.spec @@ -120,9 +120,7 @@ end { 1 => 2, - 345 => [ - 4, - ], + 345 => [4], } #~# ORIGINAL @@ -138,9 +136,7 @@ end { 1 => 2, - foo: [ - 4, - ], + foo: [4], } #~# ORIGINAL @@ -152,9 +148,7 @@ foo 1, bar: [ #~# EXPECTED -foo 1, bar: [ - 2, - ], +foo 1, bar: [2], baz: 3 #~# ORIGINAL diff --git a/spec/lib/rufo/formatter_source_specs/calls_with_receiver.rb.spec b/spec/lib/rufo/formatter_source_specs/calls_with_receiver.rb.spec index c8bfaba7..d126878f 100644 --- a/spec/lib/rufo/formatter_source_specs/calls_with_receiver.rb.spec +++ b/spec/lib/rufo/formatter_source_specs/calls_with_receiver.rb.spec @@ -160,9 +160,7 @@ foo.bar(1) #~# EXPECTED foo.bar(1) - .baz([ - 2, - ]) + .baz([2]) #~# ORIGINAL @@ -227,6 +225,7 @@ foo.bar( ) #~# ORIGINAL +#~# print_width: 5 foo 1, [ 2, @@ -237,10 +236,9 @@ foo 1, [ #~# EXPECTED foo 1, [ - 2, - - 3, -] + 2, + 3, + ] #~# ORIGINAL From 5d50eb64f7fbbd0ff36126809997c2106cf41311 Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Thu, 16 Nov 2017 10:22:20 +0100 Subject: [PATCH 21/43] Fix nested array issue --- lib/rufo/formatter.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/rufo/formatter.rb b/lib/rufo/formatter.rb index 70933e87..08b9aa64 100644 --- a/lib/rufo/formatter.rb +++ b/lib/rufo/formatter.rb @@ -409,6 +409,9 @@ def visit(node) return if doc.nil? doc = B.align(@indent, doc) puts doc.inspect + if in_doc_mode? + return doc + end @output << Rufo::DocPrinter.print_doc_to_string(doc, {print_width: print_width - @indent - (@column - @indent)})[:formatted] when :hash if in_doc_mode? From 2d7b873b9f468ee2dd6208af2402f4261f0fa0ae Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Thu, 16 Nov 2017 21:41:54 +0100 Subject: [PATCH 22/43] Add printer example for heredoc Why: So that I can workout what the formatter needs to construct. --- spec/lib/rufo/doc_printer_spec.rb | 55 +++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/spec/lib/rufo/doc_printer_spec.rb b/spec/lib/rufo/doc_printer_spec.rb index 83407fb3..d38106f9 100644 --- a/spec/lib/rufo/doc_printer_spec.rb +++ b/spec/lib/rufo/doc_printer_spec.rb @@ -171,4 +171,59 @@ def print_doc(doc, options = nil) ) end end + + context 'array with heredoc and comment' do + let(:doc) { + B.group( + B.concat([ + "[", + B.indent( + B.concat([ + B::SOFT_LINE, + B.join( + B.concat([",", B::LINE]), + [ + B.concat(['1', B.if_break(',', '')]) + ] + ), + B::LINE, + B.concat([ + "<<-EOF", + ",", + B.line_suffix(' # a comment'), + B::LINE_SUFFIX_BOUNDARY, + 'heredoc contents', + B::LINE, + 'EOF' + + ]), + B::LINE, + B.join( + B.concat([",", B::LINE]), + [ + B.concat(['2', B.if_break(',', '')]) + ] + ), + ]) + ), + B::SOFT_LINE, + "]", + ]), + should_break: true + ) + } + + it 'formats array with comment' do + expect(print(doc, print_width: 80)).to eql(<<~CODE.chomp("\n") + [ + 1, + <<-EOF, # a comment + heredoc contents + EOF + 2, + ] + CODE + ) + end + end end From 68113eab3621579d84fd04fc7d49b502ae4f07ab Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Thu, 16 Nov 2017 22:05:35 +0100 Subject: [PATCH 23/43] Remove unneeded code --- lib/rufo/formatter.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/rufo/formatter.rb b/lib/rufo/formatter.rb index 08b9aa64..bf3ed617 100644 --- a/lib/rufo/formatter.rb +++ b/lib/rufo/formatter.rb @@ -2833,7 +2833,6 @@ def visit_literal_elements_doc(elements, inside_hash: false, inside_array: false first_comment = comments.shift doc << B.concat([doc.pop, B.line_suffix(" " + first_comment.rstrip)]) end - add_comments_to_doc(comments, doc) next unless comma? next_token @@ -2850,7 +2849,6 @@ def visit_literal_elements_doc(elements, inside_hash: false, inside_array: false doc << B.concat([doc.pop, B.line_suffix(" " + first_comment.rstrip)]) end end - add_comments_to_doc(comments, doc) # We have to be careful not to aumatically write a heredoc on next_token, # because we miss the chance to write a comma to separate elements From de073df43c675b1a8f7a51cc3f6706a4f4d3e8cf Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Sat, 18 Nov 2017 20:07:54 +0100 Subject: [PATCH 24/43] Cleanup --- lib/rufo/formatter.rb | 125 +++--------------------------------------- 1 file changed, 9 insertions(+), 116 deletions(-) diff --git a/lib/rufo/formatter.rb b/lib/rufo/formatter.rb index bf3ed617..a3fdae03 100644 --- a/lib/rufo/formatter.rb +++ b/lib/rufo/formatter.rb @@ -404,15 +404,16 @@ def visit(node) when :params visit_params(node) when :array - puts node.inspect doc = visit_array(node) return if doc.nil? + doc = B.align(@indent, doc) - puts doc.inspect if in_doc_mode? return doc end - @output << Rufo::DocPrinter.print_doc_to_string(doc, {print_width: print_width - @indent - (@column - @indent)})[:formatted] + @output << Rufo::DocPrinter.print_doc_to_string( + doc, {print_width: print_width - @indent - (@column - @indent)} + )[:formatted] when :hash if in_doc_mode? capture_output { @@ -1494,24 +1495,20 @@ def visit_args_add_star_doc(node) # arg1, ..., *star doc = visit args else - pre_doc, pre_comments, post_comments, has_comment = with_doc_mode { + pre_doc, *_ = with_doc_mode { visit_literal_elements_doc(to_ary(args)) } doc.concat(pre_doc) end - # skip_space - skip_comma_and_spaces if comma? consume_op "*" - # skip_space_or_newline_doc doc << "*#{visit star}" if post_args && !post_args.empty? skip_comma_and_spaces - # visit_comma_separated_list post_args - post_doc, pre_comments, post_comments, has_comment = with_doc_mode { + post_doc, *_ = with_doc_mode { visit_literal_elements_doc(to_ary(post_args)) } doc.concat(post_doc) @@ -2179,7 +2176,6 @@ def visit_array(node) token_column = current_token_column check :on_lbracket - # write "[" next_token if elements @@ -2191,9 +2187,7 @@ def visit_array(node) end check :on_rbracket - # write "]" next_token - puts trailing_commas.inspect if trailing_commas && !doc.empty? last = doc.pop doc << B.concat([last, B.if_break(",", "")]) @@ -2778,48 +2772,11 @@ def visit_literal_elements_doc(elements, inside_hash: false, inside_array: false doc = [] pre_comments = [] post_comments = [] - has_comment = false - # base_column = @column - # base_line = @line - # needs_final_space = (inside_hash || inside_array) && space? - # first_space = skip_space - - # if inside_hash - # needs_final_space = false - # end - - # if inside_array - # needs_final_space = false - # end - - # if newline? || comment? - # needs_final_space = false - # end - - # # If there's a newline right at the beginning, - # # write it, and we'll indent element and always - # # add a trailing comma to the last element - # needs_trailing_comma = newline? || comment? - # if needs_trailing_comma - # if (call_info = @line_to_call_info[@line]) - # call_info << true - # end - - # needed_indent = next_indent - # indent { consume_end_of_line } - # write_indent(needed_indent) - # else - # needed_indent = base_column - # end - - # wrote_comma = false - # first_space = nil comments, newline_before_comment = skip_space_or_newline_doc - puts "comments 1 = #{comments}" - has_comment ||= add_comments_to_doc(comments, pre_comments) + has_comment = add_comments_to_doc(comments, pre_comments) + elements.each_with_index do |elem, i| - puts elem.inspect doc_el = visit(elem) if doc_el.is_a?(Array) doc.concat(doc_el) @@ -2827,7 +2784,6 @@ def visit_literal_elements_doc(elements, inside_hash: false, inside_array: false doc << doc_el end comments, newline_before_comment = skip_space_or_newline_doc - puts "comments 2 = #{comments}" unless comments.empty? has_comment = true first_comment = comments.shift @@ -2837,8 +2793,6 @@ def visit_literal_elements_doc(elements, inside_hash: false, inside_array: false next unless comma? next_token comments, newline_before_comment = skip_space_or_newline_doc - puts "comments 3 = #{comments}" - puts newline_before_comment unless comments.empty? has_comment = true first_comment = comments.shift @@ -2849,69 +2803,8 @@ def visit_literal_elements_doc(elements, inside_hash: false, inside_array: false doc << B.concat([doc.pop, B.line_suffix(" " + first_comment.rstrip)]) end end + end - # We have to be careful not to aumatically write a heredoc on next_token, - # because we miss the chance to write a comma to separate elements - # first_space = skip_space_no_heredoc_check - # wrote_comma = check_heredocs_in_literal_elements(is_last, needs_trailing_comma, wrote_comma) - - # next unless comma? - - # unless is_last - # write "," - # wrote_comma = true - # end - - # # We have to be careful not to aumatically write a heredoc on next_token, - # # because we miss the chance to write a comma to separate elements - # next_token_no_heredoc_check - - # first_space = skip_space_no_heredoc_check - # wrote_comma = check_heredocs_in_literal_elements(is_last, needs_trailing_comma, wrote_comma) - - # if newline? || comment? - # if is_last - # # Nothing - # else - # indent(needed_indent) do - # consume_end_of_line(first_space: first_space) - # write_indent - # end - # end - # else - # write_space unless is_last - # end - end - - # if needs_trailing_comma - # write "," unless wrote_comma || !trailing_commas - - # consume_end_of_line(first_space: first_space) - # write_indent - # elsif comment? - # consume_end_of_line(first_space: first_space) - # else - # if needs_final_space - # consume_space - # else - # skip_space_or_newline - # end - # end - - # if current_token_column == token_column && needed_indent < token_column - # # If the closing token is aligned with the opening token, we want to - # # keep it like that, for example in: - # # - # # foo([ - # # 2, - # # ]) - # @literal_indents << [base_line, @line, token_column + INDENT_SIZE - needed_indent] - # elsif call_info && call_info[0] == current_token_column - # # If the closing literal position matches the column where - # # the call started, we want to preserve it like that - # # (otherwise we align it to the first parameter) - # call_info << @line - # end [doc, pre_comments, post_comments, has_comment] end From 53e5eee099284494275e672ac5090427b1a4c059 Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Wed, 22 Nov 2017 22:11:02 +0100 Subject: [PATCH 25/43] Address PR feedback --- lib/rufo/formatter.rb | 4 +--- lib/rufo/settings.rb | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/rufo/formatter.rb b/lib/rufo/formatter.rb index a3fdae03..44d821ff 100644 --- a/lib/rufo/formatter.rb +++ b/lib/rufo/formatter.rb @@ -1495,9 +1495,7 @@ def visit_args_add_star_doc(node) # arg1, ..., *star doc = visit args else - pre_doc, *_ = with_doc_mode { - visit_literal_elements_doc(to_ary(args)) - } + pre_doc, *_ = with_doc_mode { visit_literal_elements_doc(to_ary(args)) } doc.concat(pre_doc) end diff --git a/lib/rufo/settings.rb b/lib/rufo/settings.rb index c66971fc..bae4e5d8 100644 --- a/lib/rufo/settings.rb +++ b/lib/rufo/settings.rb @@ -23,7 +23,7 @@ def init_settings(options) value = options.fetch(name, default) unless valid_options.include?(value) $stderr.puts "Invalid value for #{name}: #{value.inspect}. Valid " \ - "values are: #{valid_options.map(&:inspect).join(', ')}" + "values are: #{valid_options.map(&:inspect).join(', ')}" value = default end self.public_send("#{name}=", value) From d5bed76f4195ad37a0255ab44fb4e18a48f52f86 Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Wed, 22 Nov 2017 22:13:12 +0100 Subject: [PATCH 26/43] Add example of nested array with brackets --- .../lib/rufo/formatter_source_specs/array_literal.rb.spec | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/spec/lib/rufo/formatter_source_specs/array_literal.rb.spec b/spec/lib/rufo/formatter_source_specs/array_literal.rb.spec index bb43a0c1..907e33da 100644 --- a/spec/lib/rufo/formatter_source_specs/array_literal.rb.spec +++ b/spec/lib/rufo/formatter_source_specs/array_literal.rb.spec @@ -280,3 +280,11 @@ x = [{ *a, b, ] + +#~# ORIGINAL nested_array_with_brackets + +[([1,2]), ([3,4]), ([5,6])] + +#~# EXPECTED + +[([1,2]), ([3,4]), ([5,6])] From b57d338f8bb8ee414a038fdcc67993586df43b57 Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Thu, 23 Nov 2017 23:07:29 +0100 Subject: [PATCH 27/43] Fix parens spec --- lib/rufo/formatter.rb | 8 +++++++- .../lib/rufo/formatter_source_specs/array_literal.rb.spec | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/rufo/formatter.rb b/lib/rufo/formatter.rb index 2800c948..e3dff2b9 100644 --- a/lib/rufo/formatter.rb +++ b/lib/rufo/formatter.rb @@ -411,7 +411,13 @@ def visit(node) when :defs visit_def_with_receiver(node) when :paren - visit_paren(node) + if in_doc_mode? + capture_output { + visit_paren(node) + } + else + visit_paren(node) + end when :params visit_params(node) when :array diff --git a/spec/lib/rufo/formatter_source_specs/array_literal.rb.spec b/spec/lib/rufo/formatter_source_specs/array_literal.rb.spec index 907e33da..2434cc3e 100644 --- a/spec/lib/rufo/formatter_source_specs/array_literal.rb.spec +++ b/spec/lib/rufo/formatter_source_specs/array_literal.rb.spec @@ -287,4 +287,4 @@ x = [{ #~# EXPECTED -[([1,2]), ([3,4]), ([5,6])] +[([1, 2]), ([3, 4]), ([5, 6])] From c607c78d51e7f90136617321392855c0bf579b68 Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Mon, 27 Nov 2017 20:53:20 +0100 Subject: [PATCH 28/43] Switch from CODE to RUBY for heredocs Why: So that syntax highlighting will work in some editors. --- spec/lib/rufo/doc_printer_spec.rb | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/spec/lib/rufo/doc_printer_spec.rb b/spec/lib/rufo/doc_printer_spec.rb index d38106f9..821a4cd0 100644 --- a/spec/lib/rufo/doc_printer_spec.rb +++ b/spec/lib/rufo/doc_printer_spec.rb @@ -100,12 +100,12 @@ def print_doc(doc, options = nil) context "array expression" do let(:code_filled) { - <<~CODE.chomp("\n") + <<~RUBY.chomp("\n") [1, 2, 3, 4, 5] - CODE + RUBY } let(:code_broken) { - <<~CODE.chomp("\n") + <<~RUBY.chomp("\n") [ 1, 2, @@ -113,7 +113,7 @@ def print_doc(doc, options = nil) 4, 5, ] - CODE + RUBY } let(:doc) { @@ -163,11 +163,11 @@ def print_doc(doc, options = nil) } it 'formats array with comment' do - expect(print(doc, print_width: 80)).to eql(<<~CODE.chomp("\n") + expect(print(doc, print_width: 80)).to eql(<<~RUBY.chomp("\n") [ 1 # a comment ] - CODE + RUBY ) end end @@ -214,7 +214,7 @@ def print_doc(doc, options = nil) } it 'formats array with comment' do - expect(print(doc, print_width: 80)).to eql(<<~CODE.chomp("\n") + expect(print(doc, print_width: 80)).to eql(<<~RUBY.chomp("\n") [ 1, <<-EOF, # a comment @@ -222,7 +222,7 @@ def print_doc(doc, options = nil) EOF 2, ] - CODE + RUBY ) end end From 79449646babeaccbdfe8be07ea6ce463f6d21016 Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Mon, 27 Nov 2017 20:58:07 +0100 Subject: [PATCH 29/43] Cleanup unused code --- lib/rufo/formatter.rb | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/lib/rufo/formatter.rb b/lib/rufo/formatter.rb index e3dff2b9..6a1534d0 100644 --- a/lib/rufo/formatter.rb +++ b/lib/rufo/formatter.rb @@ -3143,7 +3143,7 @@ def skip_space_or_newline(_want_semicolon: false, write_first_semicolon: false) found_semicolon end - def skip_space_or_newline_doc(_want_semicolon: false, write_first_semicolon: false) + def skip_space_or_newline_doc found_newline = false found_comment = false found_semicolon = false @@ -3162,24 +3162,12 @@ def skip_space_or_newline_doc(_want_semicolon: false, write_first_semicolon: fal newline_before_comment = true end when :on_semicolon - if (!found_newline && !found_comment) || (!found_semicolon && write_first_semicolon) - # write "; " - end next_token last = :semicolon found_semicolon = true when :on_comment - # write_line if last == :newline - - # write_indent if found_comment if current_token_value.end_with?("\n") - # write_space - # write current_token_value.rstrip - # write "\n" - # write_indent(next_indent) @column = next_indent - else - # write current_token_value end comments << current_token_value next_token From 84f5f0ab9e25b95d46bfc6e25f562128f9e8f328 Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Sat, 2 Dec 2017 04:55:55 -0700 Subject: [PATCH 30/43] Fix printing heredocs in printer Why: content was being removed due to the regex being used. --- lib/rufo/doc_printer.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/rufo/doc_printer.rb b/lib/rufo/doc_printer.rb index 0cb89a86..fe66a7b2 100644 --- a/lib/rufo/doc_printer.rb +++ b/lib/rufo/doc_printer.rb @@ -171,9 +171,11 @@ def print_doc_to_string(doc, opts) pos = 0 else if out.length > 0 + popped = [] while out.length > 0 && (out.last =~ /^[^\S\n]*$/) - out.pop + popped << out.pop.rstrip end + out.concat(popped.reject(&:empty?)) unless out.empty? out[-1] = out.last.sub(/[^\S\n]*$/, "") From 5f97fdd67c2beb7aa1b404612a28431b392793e8 Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Sat, 2 Dec 2017 04:58:48 -0700 Subject: [PATCH 31/43] Fix heredoc handling for doc printer --- lib/rufo/formatter.rb | 207 +++++++++++++++--- .../method_calls.rb.spec | 4 +- 2 files changed, 180 insertions(+), 31 deletions(-) diff --git a/lib/rufo/formatter.rb b/lib/rufo/formatter.rb index 6a1534d0..391557d9 100644 --- a/lib/rufo/formatter.rb +++ b/lib/rufo/formatter.rb @@ -238,7 +238,7 @@ def visit(node) consume_token :on_backtick when :string_literal, :xstring_literal if in_doc_mode? - capture_output { visit_string_literal node } + capture_output { visit_string_literal node, bail_on_heredoc: true } else visit_string_literal node end @@ -605,7 +605,7 @@ def declaration?(exp) end end - def visit_string_literal(node) + def visit_string_literal(node, bail_on_heredoc: false) # [:string_literal, [:string_content, exps]] heredoc = current_token_kind == :on_heredoc_beg tilde = current_token_value.include?("~") @@ -617,6 +617,10 @@ def visit_string_literal(node) @heredocs << [node, tilde] # Get the next_token while capturing any output. # This is needed so that we can add a comma if one is not already present. + if bail_on_heredoc + next_token_no_heredoc_check + return + end captured_output = capture_output { next_token } inside_literal_elements_list = !@literal_elements_level.nil? && @@ -1181,6 +1185,28 @@ def flush_heredocs @last_was_heredoc = true if printed end + def flush_heredocs_doc + doc = [] + comment = nil + if comment? + comment = current_token_value.rstrip + next_token + end + + until @heredocs.empty? + heredoc, tilde = @heredocs.first + + @heredocs.shift + @current_heredoc = [heredoc, tilde] + doc << capture_output { visit_string_literal_end(heredoc) } + @current_heredoc = nil + printed = true + end + + @last_was_heredoc = true if printed + [doc, comment] + end + def visit_command_call(node) # [:command_call, # receiver @@ -1527,7 +1553,7 @@ def visit_args_add_star_doc(node) # arg1, ..., *star doc = visit args else - pre_doc, *_ = with_doc_mode { visit_literal_elements_doc(to_ary(args)) } + pre_doc, *_ = with_doc_mode { visit_literal_elements_simple_doc(to_ary(args)) } doc.concat(pre_doc) end @@ -1539,7 +1565,7 @@ def visit_args_add_star_doc(node) if post_args && !post_args.empty? skip_comma_and_spaces post_doc, *_ = with_doc_mode { - visit_literal_elements_doc(to_ary(post_args)) + visit_literal_elements_simple_doc(to_ary(post_args)) } doc.concat(post_doc) end @@ -2209,38 +2235,22 @@ def visit_array(node) next_token if elements - doc, pre_comments, post_comments, has_comment = with_doc_mode { + doc = with_doc_mode { visit_literal_elements_doc(to_ary(elements), inside_array: true, token_column: token_column) } else skip_space_or_newline + doc = B.group( + B.concat([ + "[", + "]", + ]) + ) end check :on_rbracket next_token - if trailing_commas && !doc.empty? - last = doc.pop - doc << B.concat([last, B.if_break(",", "")]) - end - - B.group( - B.concat([ - "[", - B.indent( - B.concat([ - B.concat(pre_comments), - B::SOFT_LINE, - B.join( - B.concat([",", B::LINE_SUFFIX_BOUNDARY, B::LINE]), - doc - ), - ]) - ), - B::SOFT_LINE, - "]", - ]), - should_break: has_comment - ) + doc end def visit_q_or_i_array(node) @@ -2802,7 +2812,7 @@ def add_comments_to_doc(comments, doc) return true end - def visit_literal_elements_doc(elements, inside_hash: false, inside_array: false, token_column: false) + def visit_literal_elements_simple_doc(elements, inside_hash: false, inside_array: false, token_column: false) doc = [] pre_comments = [] post_comments = [] @@ -2842,6 +2852,134 @@ def visit_literal_elements_doc(elements, inside_hash: false, inside_array: false [doc, pre_comments, post_comments, has_comment] end + def visit_literal_elements_doc(elements, inside_hash: false, inside_array: false, token_column: false) + doc = [] + current_doc = [] + element_doc = [] + pre_comments = [] + post_comments = [] + has_heredocs = false + + comments, newline_before_comment = skip_space_or_newline_doc + has_comment = add_comments_to_doc(comments, pre_comments) + + elements.each_with_index do |elem, i| + current_doc.concat(element_doc) + element_doc = [] + doc_el = visit(elem) + if doc_el.is_a?(Array) + element_doc.concat(doc_el) + else + element_doc << doc_el + end + heredoc_val, heredoc_comment = check_heredocs_in_literal_elements_doc + unless heredoc_val.nil? + comment_doc = heredoc_comment + + last = current_doc.pop + unless last.nil? + doc << B.join( + B.concat([",", B::LINE_SUFFIX_BOUNDARY, B::LINE]), + [ + *current_doc, + B.concat([last, B.if_break(',', '')]) + ] + ) + end + current_doc = [] + doc << B.concat([ + *element_doc, + ",", + B.line_suffix(" " + comment_doc), + B::LINE_SUFFIX_BOUNDARY, + heredoc_val.last.rstrip, + ]) + has_heredocs = true + element_doc = [] + end + comments, newline_before_comment = skip_space_or_newline_doc + unless comments.empty? + has_comment = true + first_comment = comments.shift + element_doc << B.concat([element_doc.pop, B.line_suffix(" " + first_comment.rstrip)]) + end + + next unless comma? + next_token_no_heredoc_check + heredoc_val, heredoc_comment = check_heredocs_in_literal_elements_doc + unless heredoc_val.nil? + comment_doc = nil + unless comments.empty? + comment_doc = element_doc.pop + else + comment_doc = heredoc_comment + end + + last = current_doc.pop + unless last.nil? + doc << B.join( + B.concat([",", B::LINE_SUFFIX_BOUNDARY, B::LINE]), + [ + *current_doc, + B.concat([last, B.if_break(',', '')]) + ] + ) + end + current_doc = [] + comment_array = [B.line_suffix(" " + comment_doc)] if comment_doc + comment_array ||= [] + doc << B.concat([ + *element_doc, + ",", + *comment_array, + B::LINE_SUFFIX_BOUNDARY, + heredoc_val.last.rstrip, + B::SOFT_LINE, + ]) + has_heredocs = true + element_doc = [] + end + comments, newline_before_comment = skip_space_or_newline_doc + + unless comments.empty? + has_comment = true + first_comment = comments.shift + + if newline_before_comment + element_doc << B.concat([element_doc.pop, B.line_suffix(B.concat([B::LINE, first_comment.rstrip]))]) + else + element_doc << B.concat([element_doc.pop, B.line_suffix(" " + first_comment.rstrip)]) + end + end + end + current_doc.concat(element_doc) + + if trailing_commas && !current_doc.empty? + last = current_doc.pop + current_doc << B.concat([last, B.if_break(',', '')]) + end + doc << B.join( + B.concat([",", B::LINE_SUFFIX_BOUNDARY, B::LINE]), + current_doc + ) + + B.group( + B.concat([ + "[", + B.indent( + B.concat([ + B.concat(pre_comments), + B::SOFT_LINE, + *doc, + ]) + ), + B::SOFT_LINE, + "]", + ]), + should_break: has_comment || has_heredocs + ) + end + def check_heredocs_in_literal_elements(is_last, needs_trailing_comma, wrote_comma) if (newline? || comment?) && !@heredocs.empty? if is_last && trailing_commas @@ -2854,6 +2992,14 @@ def check_heredocs_in_literal_elements(is_last, needs_trailing_comma, wrote_comm wrote_comma end + def check_heredocs_in_literal_elements_doc + skip_space + if (newline? || comment?) && !@heredocs.empty? + return flush_heredocs_doc + end + [] + end + def visit_if(node) visit_if_or_unless node, "if" end @@ -3733,6 +3879,9 @@ def next_token @tokens.pop if (newline? || comment?) && !@heredocs.empty? + if in_doc_mode? + return + end flush_heredocs end diff --git a/spec/lib/rufo/formatter_source_specs/method_calls.rb.spec b/spec/lib/rufo/formatter_source_specs/method_calls.rb.spec index d7d34de2..2a28ee35 100644 --- a/spec/lib/rufo/formatter_source_specs/method_calls.rb.spec +++ b/spec/lib/rufo/formatter_source_specs/method_calls.rb.spec @@ -513,10 +513,10 @@ EOF #~# EXPECTED foo 1, [ - <<-EOF, + <<-EOF, bar EOF -] + ] #~# ORIGINAL From e71def654bc79bac9284fcd58a9f0e0850a0e43f Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Sat, 2 Dec 2017 05:23:10 -0700 Subject: [PATCH 32/43] Add new test cases and fix them --- lib/rufo/formatter.rb | 24 ++++++++++++++++--- .../array_literal.rb.spec | 24 +++++++++++++++++++ 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/lib/rufo/formatter.rb b/lib/rufo/formatter.rb index 391557d9..4b88ce63 100644 --- a/lib/rufo/formatter.rb +++ b/lib/rufo/formatter.rb @@ -329,7 +329,13 @@ def visit(node) when :massign visit_multiple_assign(node) when :ifop - visit_ternary_if(node) + if in_doc_mode? + capture_output { + visit_ternary_if(node) + } + else + visit_ternary_if(node) + end when :if_mod visit_suffix(node, "if") when :unless_mod @@ -363,7 +369,13 @@ def visit(node) visit_comma_separated_list node[1] end when :method_add_arg - visit_call_without_receiver(node) + if in_doc_mode? + capture_output { + visit_call_without_receiver(node) + } + else + visit_call_without_receiver(node) + end when :method_add_block visit_call_with_block(node) when :call @@ -455,7 +467,13 @@ def visit(node) when :regexp_literal visit_regexp_literal(node) when :aref - visit_array_access(node) + if in_doc_mode? + capture_output { + visit_array_access(node) + } + else + visit_array_access(node) + end when :aref_field visit_array_setter(node) when :sclass diff --git a/spec/lib/rufo/formatter_source_specs/array_literal.rb.spec b/spec/lib/rufo/formatter_source_specs/array_literal.rb.spec index 2434cc3e..04157e9c 100644 --- a/spec/lib/rufo/formatter_source_specs/array_literal.rb.spec +++ b/spec/lib/rufo/formatter_source_specs/array_literal.rb.spec @@ -288,3 +288,27 @@ x = [{ #~# EXPECTED [([1, 2]), ([3, 4]), ([5, 6])] + +#~# ORIGINAL array_with_method_call + +[a(1)] + +#~# EXPECTED + +[a(1)] + +#~# ORIGINAL array_with_ternary + +[true ? 1 : 2] + +#~# EXPECTED + +[true ? 1 : 2] + +#~# ORIGINAL array_with_indexing_element + +[a[:b]] + +#~# EXPECTED + +[a[:b]] From 711bfd59a67e408542d0a9e0e2003aad46306467 Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Sat, 2 Dec 2017 06:11:21 -0700 Subject: [PATCH 33/43] Refactor to handle non doc handled nodes generically Why: So that each type does not need to be wrapped in a capture_output block. --- lib/rufo/formatter.rb | 94 ++++++++++++++++++------------------------- 1 file changed, 40 insertions(+), 54 deletions(-) diff --git a/lib/rufo/formatter.rb b/lib/rufo/formatter.rb index 4b88ce63..6db06779 100644 --- a/lib/rufo/formatter.rb +++ b/lib/rufo/formatter.rb @@ -190,7 +190,26 @@ def visit(node) unless node.is_a?(Array) bug "unexpected node: #{node} at #{current_token}" end + result = visit_doc(node) + if result != false + if in_doc_mode? + return result + end + return if result.nil? + @output << Rufo::DocPrinter.print_doc_to_string( + result, {print_width: print_width - @indent - (@column - @indent)} + )[:formatted] + return + end + if in_doc_mode? + capture_output { visit_non_doc(node) } + else + visit_non_doc(node) + end + end + + def visit_non_doc(node) case node.first when :program # Topmost node @@ -237,12 +256,7 @@ def visit(node) # [:@backtick, "`", [1, 4]] consume_token :on_backtick when :string_literal, :xstring_literal - if in_doc_mode? - capture_output { visit_string_literal node, bail_on_heredoc: true } - else - visit_string_literal node - end - + visit_string_literal node when :string_concat visit_string_concat node when :@tstring_content @@ -329,13 +343,7 @@ def visit(node) when :massign visit_multiple_assign(node) when :ifop - if in_doc_mode? - capture_output { - visit_ternary_if(node) - } - else - visit_ternary_if(node) - end + visit_ternary_if(node) when :if_mod visit_suffix(node, "if") when :unless_mod @@ -369,13 +377,7 @@ def visit(node) visit_comma_separated_list node[1] end when :method_add_arg - if in_doc_mode? - capture_output { - visit_call_without_receiver(node) - } - else - visit_call_without_receiver(node) - end + visit_call_without_receiver(node) when :method_add_block visit_call_with_block(node) when :call @@ -423,35 +425,11 @@ def visit(node) when :defs visit_def_with_receiver(node) when :paren - if in_doc_mode? - capture_output { - visit_paren(node) - } - else - visit_paren(node) - end + visit_paren(node) when :params visit_params(node) - when :array - doc = visit_array(node) - return if doc.nil? - - doc = B.align(@indent, doc) - if in_doc_mode? - return doc - end - @output << Rufo::DocPrinter.print_doc_to_string( - doc, {print_width: print_width - @indent - (@column - @indent)} - )[:formatted] when :hash - if in_doc_mode? - capture_output { - visit_hash(node) - } - else - visit_hash(node) - end - + visit_hash(node) when :assoc_new visit_hash_key_value(node) when :assoc_splat @@ -467,13 +445,7 @@ def visit(node) when :regexp_literal visit_regexp_literal(node) when :aref - if in_doc_mode? - capture_output { - visit_array_access(node) - } - else - visit_array_access(node) - end + visit_array_access(node) when :aref_field visit_array_setter(node) when :sclass @@ -534,6 +506,21 @@ def visit(node) @node_level -= 1 end + def visit_doc(node) + case node.first + when :array + doc = visit_array(node) + return if doc.nil? + + return B.align(@indent, doc) + when :args_add_star + return visit_args_add_star_doc(node) if in_doc_mode? + return false + else + return false + end + end + def visit_exps(exps, with_indent: false, with_lines: true, want_trailing_multiline: false) consume_end_of_line(at_prefix: true) @@ -1531,7 +1518,6 @@ def visit_call_args(node) end def visit_args_add_star(node) - return visit_args_add_star_doc(node) if in_doc_mode? # [:args_add_star, args, star, post_args] _, args, star, *post_args = node From 7fed281e119d86f3f4114f22f6406dd56ed83185 Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Sat, 2 Dec 2017 06:14:19 -0700 Subject: [PATCH 34/43] Fix print_width config calculation Why: Until this is used by other parts of the formatter we cannot handle anything before the array other than indentation. --- lib/rufo/formatter.rb | 2 +- .../array_literal.rb.spec | 8 +++ .../method_calls.rb.spec | 60 +++++++++++-------- 3 files changed, 45 insertions(+), 25 deletions(-) diff --git a/lib/rufo/formatter.rb b/lib/rufo/formatter.rb index 6db06779..dee1a805 100644 --- a/lib/rufo/formatter.rb +++ b/lib/rufo/formatter.rb @@ -197,7 +197,7 @@ def visit(node) end return if result.nil? @output << Rufo::DocPrinter.print_doc_to_string( - result, {print_width: print_width - @indent - (@column - @indent)} + result, {print_width: print_width - @indent} )[:formatted] return end diff --git a/spec/lib/rufo/formatter_source_specs/array_literal.rb.spec b/spec/lib/rufo/formatter_source_specs/array_literal.rb.spec index 04157e9c..79d8fae9 100644 --- a/spec/lib/rufo/formatter_source_specs/array_literal.rb.spec +++ b/spec/lib/rufo/formatter_source_specs/array_literal.rb.spec @@ -312,3 +312,11 @@ x = [{ #~# EXPECTED [a[:b]] + +#~# ORIGINAL array_with_symbol + +[:a] + +#~# EXPECTED + +[:a] diff --git a/spec/lib/rufo/formatter_source_specs/method_calls.rb.spec b/spec/lib/rufo/formatter_source_specs/method_calls.rb.spec index 2a28ee35..7a20e308 100644 --- a/spec/lib/rufo/formatter_source_specs/method_calls.rb.spec +++ b/spec/lib/rufo/formatter_source_specs/method_calls.rb.spec @@ -490,7 +490,6 @@ foo 1, { } #~# ORIGINAL -#~# print_width: 7 foo 1, [ 1, @@ -498,9 +497,7 @@ foo 1, [ #~# EXPECTED -foo 1, [ - 1, - ] +foo 1, [1] #~# ORIGINAL @@ -585,7 +582,6 @@ foo(& block) foo(&block) #~# ORIGINAL -#~# print_width: 7 foo 1, [ 2, @@ -593,12 +589,9 @@ foo 1, [ #~# EXPECTED -foo 1, [ - 2, - ] +foo 1, [2] #~# ORIGINAL -#~# print_width: 7 foo 1, [ 2, @@ -606,9 +599,7 @@ foo 1, [ #~# EXPECTED -foo 1, [ - 2, - ] +foo 1, [2] #~# ORIGINAL @@ -683,7 +674,6 @@ begin end #~# ORIGINAL -#~# print_width: 6 foo([ 1, @@ -691,12 +681,9 @@ foo([ #~# EXPECTED -foo([ - 1, -]) +foo([1]) #~# ORIGINAL -#~# print_width: 7 begin foo([ @@ -707,13 +694,10 @@ end #~# EXPECTED begin - foo([ - 1, - ]) + foo([1]) end #~# ORIGINAL -#~# print_width: 9 (a b).c([ 1, @@ -721,9 +705,7 @@ end #~# EXPECTED -(a b).c([ - 1, -]) +(a b).c([1]) #~# ORIGINAL @@ -736,3 +718,33 @@ foobar 1, foobar 1, "foo bar" + +#~# ORIGINAL method_array_arg_print_width_break +#~# print_width: 8 + +(a b).c([ + 1, + 2, + 3, +]) + +#~# EXPECTED + +(a b).c([ + 1, + 2, + 3, +]) + +#~# ORIGINAL method_array_arg_print_width_fill +#~# print_width: 9 + +(a b).c([ + 1, + 2, + 3, +]) + +#~# EXPECTED + +(a b).c([1, 2, 3]) From e25138d5ced6a98156d1f83a9c6bdc6367cefa11 Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Sat, 2 Dec 2017 06:33:00 -0700 Subject: [PATCH 35/43] Format codebase --- lib/rufo/doc_builder.rb | 7 +- lib/rufo/doc_printer.rb | 8 +- lib/rufo/formatter.rb | 30 +++----- lib/rufo/settings.rb | 4 +- spec/lib/rufo/doc_builder_spec.rb | 7 +- spec/lib/rufo/doc_printer_spec.rb | 120 ++++++++++++++---------------- 6 files changed, 75 insertions(+), 101 deletions(-) diff --git a/lib/rufo/doc_builder.rb b/lib/rufo/doc_builder.rb index 200dc440..706086ca 100644 --- a/lib/rufo/doc_builder.rb +++ b/lib/rufo/doc_builder.rb @@ -80,7 +80,7 @@ def join(sep, arr) def add_alignment_to_doc(doc, size, tab_width) return doc unless size > 0 - (size/tab_width).times { doc = indent(doc) } + (size / tab_width).times { doc = indent(doc) } doc = align(size % tab_width, doc) align(-Float::INFINITY, doc) end @@ -112,10 +112,7 @@ def assert_doc(val) HARD_LINE = concat([{type: :line, hard: true}, BREAK_PARENT]) # This is a newline that is always included and does not cause the # indentation to change subsequently. - LITERAL_LINE = concat([ - {type: :line, hard: true, literal: true}, - BREAK_PARENT, - ]) + LITERAL_LINE = concat([{type: :line, hard: true, literal: true}, BREAK_PARENT]) # This keeps track of the cursor in the document. CURSOR = {type: :cursor, placeholder: :cursor} diff --git a/lib/rufo/doc_printer.rb b/lib/rufo/doc_printer.rb index fe66a7b2..3bd09ccc 100644 --- a/lib/rufo/doc_printer.rb +++ b/lib/rufo/doc_printer.rb @@ -107,7 +107,9 @@ def print_doc_to_string(doc, opts) second_content = parts[2] first_and_second_content_flat_cmd = [ - ind, MODE_FLAT, DocBuilder::concat([content, whitespace, second_content]), + ind, + MODE_FLAT, + DocBuilder::concat([content, whitespace, second_content]), ] first_and_second_content_fits = fits( first_and_second_content_flat_cmd, @@ -195,8 +197,8 @@ def print_doc_to_string(doc, opts) cursor_place_holder_index = out.index(DocBuilder::CURSOR[:placeholder]) if cursor_place_holder_index - before_cursor = out[0..(cursor_place_holder_index-1)].join("") - after_cursor = out[(cursor_place_holder_index+1)..-1].join("") + before_cursor = out[0..(cursor_place_holder_index - 1)].join("") + after_cursor = out[(cursor_place_holder_index + 1)..-1].join("") return { formatted: before_cursor + after_cursor, diff --git a/lib/rufo/formatter.rb b/lib/rufo/formatter.rb index dee1a805..cbc0d5c6 100644 --- a/lib/rufo/formatter.rb +++ b/lib/rufo/formatter.rb @@ -2245,10 +2245,7 @@ def visit_array(node) else skip_space_or_newline doc = B.group( - B.concat([ - "[", - "]", - ]) + B.concat(["[", "]"]) ) end @@ -2884,10 +2881,7 @@ def visit_literal_elements_doc(elements, inside_hash: false, inside_array: false unless last.nil? doc << B.join( B.concat([",", B::LINE_SUFFIX_BOUNDARY, B::LINE]), - [ - *current_doc, - B.concat([last, B.if_break(',', '')]) - ] + [*current_doc, B.concat([last, B.if_break(',', '')])] ) end current_doc = [] @@ -2923,10 +2917,7 @@ def visit_literal_elements_doc(elements, inside_hash: false, inside_array: false unless last.nil? doc << B.join( B.concat([",", B::LINE_SUFFIX_BOUNDARY, B::LINE]), - [ - *current_doc, - B.concat([last, B.if_break(',', '')]) - ] + [*current_doc, B.concat([last, B.if_break(',', '')])] ) end current_doc = [] @@ -2950,7 +2941,10 @@ def visit_literal_elements_doc(elements, inside_hash: false, inside_array: false first_comment = comments.shift if newline_before_comment - element_doc << B.concat([element_doc.pop, B.line_suffix(B.concat([B::LINE, first_comment.rstrip]))]) + element_doc << B.concat([ + element_doc.pop, + B.line_suffix(B.concat([B::LINE, first_comment.rstrip])), + ]) else element_doc << B.concat([element_doc.pop, B.line_suffix(" " + first_comment.rstrip)]) end @@ -2970,17 +2964,11 @@ def visit_literal_elements_doc(elements, inside_hash: false, inside_array: false B.group( B.concat([ "[", - B.indent( - B.concat([ - B.concat(pre_comments), - B::SOFT_LINE, - *doc, - ]) - ), + B.indent(B.concat([B.concat(pre_comments), B::SOFT_LINE, *doc])), B::SOFT_LINE, "]", ]), - should_break: has_comment || has_heredocs + should_break: has_comment || has_heredocs, ) end diff --git a/lib/rufo/settings.rb b/lib/rufo/settings.rb index bae4e5d8..ee45a10d 100644 --- a/lib/rufo/settings.rb +++ b/lib/rufo/settings.rb @@ -4,11 +4,11 @@ module Rufo::Settings align_case_when: [false, true], align_chained_calls: [false, true], trailing_commas: [true, false], - print_width: (1..Float::INFINITY) + print_width: (1..Float::INFINITY), } DEFAULT_VALUES = { - print_width: 80 + print_width: 80, } attr_accessor *OPTIONS.keys diff --git a/spec/lib/rufo/doc_builder_spec.rb b/spec/lib/rufo/doc_builder_spec.rb index 958766aa..c5da5f26 100644 --- a/spec/lib/rufo/doc_builder_spec.rb +++ b/spec/lib/rufo/doc_builder_spec.rb @@ -10,12 +10,7 @@ end it 'raises an error if a document is not the correct type' do - invalid_docs = [ - [], - 1, - {}, - {type: "string"}, - ] + invalid_docs = [[], 1, {}, {type: "string"}] invalid_docs.each do |doc| expect { described_class.concat([doc]) }.to raise_error( Rufo::DocBuilder::InvalidDocError diff --git a/spec/lib/rufo/doc_printer_spec.rb b/spec/lib/rufo/doc_printer_spec.rb index 821a4cd0..03307abf 100644 --- a/spec/lib/rufo/doc_printer_spec.rb +++ b/spec/lib/rufo/doc_printer_spec.rb @@ -115,23 +115,18 @@ def print_doc(doc, options = nil) ] RUBY } - + let(:inner_doc) { + B.concat([ + B::SOFT_LINE, + B.join( + B.concat([",", B::LINE]), + ["1", "2", "3", "4", B.concat(["5", B.if_break(",", "")])] + ), + ]) + } let(:doc) { B.group( - B.concat([ - "[", - B.indent( - B.concat([ - B::SOFT_LINE, - B.join( - B.concat([",", B::LINE]), - ["1", "2", "3", "4", B.concat(["5", B.if_break(",", "")])] - ), - ]) - ), - B::SOFT_LINE, - "]", - ]) + B.concat(["[", B.indent(inner_doc), B::SOFT_LINE, "]"]) ) } @@ -147,28 +142,30 @@ def print_doc(doc, options = nil) B.concat([ "[", B.indent( - B.concat([ - B::SOFT_LINE, - B.join( - B.concat([",", B::LINE]), - [B.concat(["1", B.line_suffix(' # a comment'), B::LINE_SUFFIX_BOUNDARY])] - ), - ]) + B.concat([ + B::SOFT_LINE, + B.join( + B.concat([",", B::LINE]), + [ + B.concat(["1", B.line_suffix(' # a comment'), B::LINE_SUFFIX_BOUNDARY]), + ] ), + ]) + ), B::SOFT_LINE, "]", ]), - should_break: true + should_break: true, ) } it 'formats array with comment' do expect(print(doc, print_width: 80)).to eql(<<~RUBY.chomp("\n") - [ - 1 # a comment - ] - RUBY - ) + [ + 1 # a comment + ] + RUBY +) end end @@ -178,52 +175,47 @@ def print_doc(doc, options = nil) B.concat([ "[", B.indent( + B.concat([ + B::SOFT_LINE, + B.join( + B.concat([",", B::LINE]), + [B.concat(['1', B.if_break(',', '')])] + ), + B::LINE, B.concat([ - B::SOFT_LINE, - B.join( - B.concat([",", B::LINE]), - [ - B.concat(['1', B.if_break(',', '')]) - ] - ), - B::LINE, - B.concat([ - "<<-EOF", - ",", - B.line_suffix(' # a comment'), - B::LINE_SUFFIX_BOUNDARY, - 'heredoc contents', - B::LINE, - 'EOF' - - ]), - B::LINE, - B.join( - B.concat([",", B::LINE]), - [ - B.concat(['2', B.if_break(',', '')]) - ] - ), - ]) + "<<-EOF", + ",", + B.line_suffix(' # a comment'), + B::LINE_SUFFIX_BOUNDARY, + 'heredoc contents', + B::LINE, + 'EOF', + ]), + B::LINE, + B.join( + B.concat([",", B::LINE]), + [B.concat(['2', B.if_break(',', '')])] ), + ]) + ), B::SOFT_LINE, "]", ]), - should_break: true + should_break: true, ) } it 'formats array with comment' do expect(print(doc, print_width: 80)).to eql(<<~RUBY.chomp("\n") - [ - 1, - <<-EOF, # a comment - heredoc contents - EOF - 2, - ] - RUBY - ) + [ + 1, + <<-EOF, # a comment + heredoc contents + EOF + 2, + ] + RUBY +) end end end From 8cc334db4e14bd66b9dd6ca0bfe49d2c7a4dfb3e Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Sat, 2 Dec 2017 06:56:16 -0700 Subject: [PATCH 36/43] Remove usage of squiggly heredocs Why: So that tests work on ruby 2.3. --- spec/lib/rufo/doc_printer_spec.rb | 34 ++++++------------------------- 1 file changed, 6 insertions(+), 28 deletions(-) diff --git a/spec/lib/rufo/doc_printer_spec.rb b/spec/lib/rufo/doc_printer_spec.rb index 03307abf..14e9402f 100644 --- a/spec/lib/rufo/doc_printer_spec.rb +++ b/spec/lib/rufo/doc_printer_spec.rb @@ -100,20 +100,10 @@ def print_doc(doc, options = nil) context "array expression" do let(:code_filled) { - <<~RUBY.chomp("\n") - [1, 2, 3, 4, 5] - RUBY + "[1, 2, 3, 4, 5]" } let(:code_broken) { - <<~RUBY.chomp("\n") - [ - 1, - 2, - 3, - 4, - 5, - ] - RUBY + "[\n 1,\n 2,\n 3,\n 4,\n 5,\n]" } let(:inner_doc) { B.concat([ @@ -160,12 +150,7 @@ def print_doc(doc, options = nil) } it 'formats array with comment' do - expect(print(doc, print_width: 80)).to eql(<<~RUBY.chomp("\n") - [ - 1 # a comment - ] - RUBY -) + expect(print(doc, print_width: 80)).to eql("[\n 1 # a comment\n]") end end @@ -206,16 +191,9 @@ def print_doc(doc, options = nil) } it 'formats array with comment' do - expect(print(doc, print_width: 80)).to eql(<<~RUBY.chomp("\n") - [ - 1, - <<-EOF, # a comment - heredoc contents - EOF - 2, - ] - RUBY -) + expect(print(doc, print_width: 80)).to eql( + "[\n 1,\n <<-EOF, # a comment\n heredoc contents\n EOF\n 2,\n]" + ) end end end From ba42b5c11835b4cbd1c2fdb0f902384691ec1826 Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Sat, 2 Dec 2017 07:05:56 -0700 Subject: [PATCH 37/43] Add test for indentation issue --- .../method_calls.rb.spec | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/spec/lib/rufo/formatter_source_specs/method_calls.rb.spec b/spec/lib/rufo/formatter_source_specs/method_calls.rb.spec index 7a20e308..0ed4ca9f 100644 --- a/spec/lib/rufo/formatter_source_specs/method_calls.rb.spec +++ b/spec/lib/rufo/formatter_source_specs/method_calls.rb.spec @@ -748,3 +748,21 @@ foobar 1, #~# EXPECTED (a b).c([1, 2, 3]) + +#~# ORIGINAL indent_bug +#~# print_width: 1 +#~# PENDING + +a([ + b( + something + ), +]) + +#~# EXPECTED + +a([ + b( + something + ), +]) From 46ea2f4a48b5bae3333ceb841c70297c651c713e Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Sat, 2 Dec 2017 10:43:46 -0700 Subject: [PATCH 38/43] Refactor literal elements method Why: to reduce code duplication. --- lib/rufo/formatter.rb | 106 ++++++++++++++++++------------------------ 1 file changed, 44 insertions(+), 62 deletions(-) diff --git a/lib/rufo/formatter.rb b/lib/rufo/formatter.rb index 6fd4a2ba..b46b6901 100644 --- a/lib/rufo/formatter.rb +++ b/lib/rufo/formatter.rb @@ -2230,23 +2230,16 @@ def visit_array(node) _, elements = node doc = [] - pre_comments = [] - post_comments = [] - has_comment = false - token_column = current_token_column - check :on_lbracket next_token if elements doc = with_doc_mode { - visit_literal_elements_doc(to_ary(elements), inside_array: true, token_column: token_column) + visit_literal_elements_doc(to_ary(elements)) } else skip_space_or_newline - doc = B.group( - B.concat(["[", "]"]) - ) + doc = "[]" end check :on_rbracket @@ -2865,7 +2858,40 @@ def visit_literal_elements_simple_doc(elements, inside_hash: false, inside_array [doc, pre_comments, post_comments, has_comment] end - def visit_literal_elements_doc(elements, inside_hash: false, inside_array: false, token_column: false) + def add_heredoc_to_doc(doc, current_doc, element_doc, comments) + value, comment = check_heredocs_in_literal_elements_doc + return [current_doc, false, element_doc] if value.nil? + + last = current_doc.pop + unless last.nil? + doc << B.join( + B.concat([",", B::LINE_SUFFIX_BOUNDARY, B::LINE]), + [*current_doc, B.concat([last, B.if_break(',', '')])] + ) + end + + comment_doc = nil + unless comments.empty? + comment_doc = element_doc.pop + else + comment_doc = comment + end + + comment_array = [B.line_suffix(" " + comment_doc)] if comment_doc + comment_array ||= [] + + doc << B.concat([ + *element_doc, + ",", + *comment_array, + B::LINE_SUFFIX_BOUNDARY, + value.last.rstrip, + B::SOFT_LINE, + ]) + return [[], true, []] + end + + def visit_literal_elements_doc(elements) doc = [] current_doc = [] element_doc = [] @@ -2885,28 +2911,10 @@ def visit_literal_elements_doc(elements, inside_hash: false, inside_array: false else element_doc << doc_el end - heredoc_val, heredoc_comment = check_heredocs_in_literal_elements_doc - unless heredoc_val.nil? - comment_doc = heredoc_comment - - last = current_doc.pop - unless last.nil? - doc << B.join( - B.concat([",", B::LINE_SUFFIX_BOUNDARY, B::LINE]), - [*current_doc, B.concat([last, B.if_break(',', '')])] - ) - end - current_doc = [] - doc << B.concat([ - *element_doc, - ",", - B.line_suffix(" " + comment_doc), - B::LINE_SUFFIX_BOUNDARY, - heredoc_val.last.rstrip, - ]) - has_heredocs = true - element_doc = [] - end + current_doc, heredoc_present, element_doc = add_heredoc_to_doc( + doc, current_doc, element_doc, [] + ) + has_heredocs ||= heredoc_present comments, newline_before_comment = skip_space_or_newline_doc unless comments.empty? has_comment = true @@ -2916,36 +2924,10 @@ def visit_literal_elements_doc(elements, inside_hash: false, inside_array: false next unless comma? next_token_no_heredoc_check - heredoc_val, heredoc_comment = check_heredocs_in_literal_elements_doc - unless heredoc_val.nil? - comment_doc = nil - unless comments.empty? - comment_doc = element_doc.pop - else - comment_doc = heredoc_comment - end - - last = current_doc.pop - unless last.nil? - doc << B.join( - B.concat([",", B::LINE_SUFFIX_BOUNDARY, B::LINE]), - [*current_doc, B.concat([last, B.if_break(',', '')])] - ) - end - current_doc = [] - comment_array = [B.line_suffix(" " + comment_doc)] if comment_doc - comment_array ||= [] - doc << B.concat([ - *element_doc, - ",", - *comment_array, - B::LINE_SUFFIX_BOUNDARY, - heredoc_val.last.rstrip, - B::SOFT_LINE, - ]) - has_heredocs = true - element_doc = [] - end + current_doc, heredoc_present, element_doc = add_heredoc_to_doc( + doc, current_doc, element_doc, comments + ) + has_heredocs ||= heredoc_present comments, newline_before_comment = skip_space_or_newline_doc unless comments.empty? From 99901fd444529223087574247d375b8a620e286d Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Sat, 2 Dec 2017 11:05:07 -0700 Subject: [PATCH 39/43] Remove unneeded code --- lib/rufo/formatter.rb | 46 +++++++++++-------------------------------- 1 file changed, 11 insertions(+), 35 deletions(-) diff --git a/lib/rufo/formatter.rb b/lib/rufo/formatter.rb index b46b6901..b2818078 100644 --- a/lib/rufo/formatter.rb +++ b/lib/rufo/formatter.rb @@ -1557,7 +1557,7 @@ def visit_args_add_star_doc(node) # arg1, ..., *star doc = visit args else - pre_doc, *_ = with_doc_mode { visit_literal_elements_simple_doc(to_ary(args)) } + pre_doc = with_doc_mode { visit_literal_elements_simple_doc(args) } doc.concat(pre_doc) end @@ -1568,9 +1568,7 @@ def visit_args_add_star_doc(node) if post_args && !post_args.empty? skip_comma_and_spaces - post_doc, *_ = with_doc_mode { - visit_literal_elements_simple_doc(to_ary(post_args)) - } + post_doc = with_doc_mode { visit_literal_elements_simple_doc(post_args) } doc.concat(post_doc) end doc @@ -2818,44 +2816,23 @@ def add_comments_to_doc(comments, doc) return true end - def visit_literal_elements_simple_doc(elements, inside_hash: false, inside_array: false, token_column: false) + # Handles literal elements where there are no comments or heredocs to worry + # about. + def visit_literal_elements_simple_doc(elements) doc = [] - pre_comments = [] - post_comments = [] - comments, newline_before_comment = skip_space_or_newline_doc - has_comment = add_comments_to_doc(comments, pre_comments) - - elements.each_with_index do |elem, i| + skip_space_or_newline + elements.each do |elem| doc_el = visit(elem) - if doc_el.is_a?(Array) - doc.concat(doc_el) - else - doc << doc_el - end - comments, newline_before_comment = skip_space_or_newline_doc - unless comments.empty? - has_comment = true - first_comment = comments.shift - doc << B.concat([doc.pop, B.line_suffix(" " + first_comment.rstrip)]) - end + doc.concat(Array(doc_el)) + skip_space_or_newline next unless comma? next_token - comments, newline_before_comment = skip_space_or_newline_doc - unless comments.empty? - has_comment = true - first_comment = comments.shift - - if newline_before_comment - doc << B.concat([doc.pop, B.line_suffix(B.concat([B::LINE, first_comment.rstrip]))]) - else - doc << B.concat([doc.pop, B.line_suffix(" " + first_comment.rstrip)]) - end - end + skip_space_or_newline end - [doc, pre_comments, post_comments, has_comment] + doc end def add_heredoc_to_doc(doc, current_doc, element_doc, comments) @@ -2896,7 +2873,6 @@ def visit_literal_elements_doc(elements) current_doc = [] element_doc = [] pre_comments = [] - post_comments = [] has_heredocs = false comments, newline_before_comment = skip_space_or_newline_doc From 660e797cfc08a60767e3c9e19a6fa2a5f1b0aeeb Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Sat, 2 Dec 2017 11:22:37 -0700 Subject: [PATCH 40/43] Refactor comment handling --- lib/rufo/formatter.rb | 42 +++++++++++++++++++----------------------- 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/lib/rufo/formatter.rb b/lib/rufo/formatter.rb index b2818078..3220c239 100644 --- a/lib/rufo/formatter.rb +++ b/lib/rufo/formatter.rb @@ -2816,6 +2816,21 @@ def add_comments_to_doc(comments, doc) return true end + def add_comments_on_line(element_doc, comments, newline_before_comment:) + return false if comments.empty? + first_comment = comments.shift + + if newline_before_comment + element_doc << B.concat([ + element_doc.pop, + B.line_suffix(B.concat([B::LINE, first_comment.rstrip])), + ]) + else + element_doc << B.concat([element_doc.pop, B.line_suffix(" " + first_comment.rstrip)]) + end + true + end + # Handles literal elements where there are no comments or heredocs to worry # about. def visit_literal_elements_simple_doc(elements) @@ -2847,14 +2862,11 @@ def add_heredoc_to_doc(doc, current_doc, element_doc, comments) ) end - comment_doc = nil unless comments.empty? - comment_doc = element_doc.pop - else - comment_doc = comment + comment = element_doc.pop end - comment_array = [B.line_suffix(" " + comment_doc)] if comment_doc + comment_array = [B.line_suffix(" " + comment)] if comment comment_array ||= [] doc << B.concat([ @@ -2892,11 +2904,7 @@ def visit_literal_elements_doc(elements) ) has_heredocs ||= heredoc_present comments, newline_before_comment = skip_space_or_newline_doc - unless comments.empty? - has_comment = true - first_comment = comments.shift - element_doc << B.concat([element_doc.pop, B.line_suffix(" " + first_comment.rstrip)]) - end + has_comment = true if add_comments_on_line(element_doc, comments, newline_before_comment: false) next unless comma? next_token_no_heredoc_check @@ -2906,19 +2914,7 @@ def visit_literal_elements_doc(elements) has_heredocs ||= heredoc_present comments, newline_before_comment = skip_space_or_newline_doc - unless comments.empty? - has_comment = true - first_comment = comments.shift - - if newline_before_comment - element_doc << B.concat([ - element_doc.pop, - B.line_suffix(B.concat([B::LINE, first_comment.rstrip])), - ]) - else - element_doc << B.concat([element_doc.pop, B.line_suffix(" " + first_comment.rstrip)]) - end - end + has_comment = true if add_comments_on_line(element_doc, comments, newline_before_comment: newline_before_comment) end current_doc.concat(element_doc) From 7ff6caf800bca17f41e78b182d0180fe769ee2e6 Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Sat, 2 Dec 2017 11:31:42 -0700 Subject: [PATCH 41/43] Code style tweaks --- lib/rufo/formatter.rb | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/lib/rufo/formatter.rb b/lib/rufo/formatter.rb index 3220c239..1e2b062b 100644 --- a/lib/rufo/formatter.rb +++ b/lib/rufo/formatter.rb @@ -515,10 +515,8 @@ def visit_doc(node) return B.align(@indent, doc) when :args_add_star return visit_args_add_star_doc(node) if in_doc_mode? - return false - else - return false end + false end def visit_exps(exps, with_indent: false, with_lines: true, want_trailing_multiline: false) @@ -2232,9 +2230,7 @@ def visit_array(node) next_token if elements - doc = with_doc_mode { - visit_literal_elements_doc(to_ary(elements)) - } + doc = with_doc_mode { visit_literal_elements_doc(to_ary(elements)) } else skip_space_or_newline doc = "[]" From c7028c2a0ff0c6ea16217827269fe911b4407903 Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Sat, 2 Dec 2017 12:05:23 -0700 Subject: [PATCH 42/43] Fix array literal bug --- lib/rufo/formatter.rb | 6 +++++- .../lib/rufo/formatter_source_specs/array_literal.rb.spec | 8 ++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/rufo/formatter.rb b/lib/rufo/formatter.rb index 1e2b062b..d9d92b09 100644 --- a/lib/rufo/formatter.rb +++ b/lib/rufo/formatter.rb @@ -2835,7 +2835,11 @@ def visit_literal_elements_simple_doc(elements) skip_space_or_newline elements.each do |elem| doc_el = visit(elem) - doc.concat(Array(doc_el)) + if doc_el.is_a?(Array) + doc.concat(doc_el) + else + doc << doc_el + end skip_space_or_newline next unless comma? diff --git a/spec/lib/rufo/formatter_source_specs/array_literal.rb.spec b/spec/lib/rufo/formatter_source_specs/array_literal.rb.spec index 79d8fae9..9ff92818 100644 --- a/spec/lib/rufo/formatter_source_specs/array_literal.rb.spec +++ b/spec/lib/rufo/formatter_source_specs/array_literal.rb.spec @@ -320,3 +320,11 @@ x = [{ #~# EXPECTED [:a] + +#~# ORIGINAL nested_array_splat + +[[], *a] + +#~# EXPECTED + +[[], *a] From 4835c73a7a968f2e28e675ddaa820d0b946d85f0 Mon Sep 17 00:00:00 2001 From: Max Brosnahan Date: Sat, 2 Dec 2017 12:18:09 -0700 Subject: [PATCH 43/43] Handle percent array nested inside normal array --- lib/rufo/formatter.rb | 3 +-- .../lib/rufo/formatter_source_specs/array_literal.rb.spec | 8 ++++++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/rufo/formatter.rb b/lib/rufo/formatter.rb index d9d92b09..c7c4cfdd 100644 --- a/lib/rufo/formatter.rb +++ b/lib/rufo/formatter.rb @@ -2219,8 +2219,7 @@ def visit_array(node) # Check if it's `%w(...)` or `%i(...)` case current_token_kind when :on_qwords_beg, :on_qsymbols_beg, :on_words_beg, :on_symbols_beg - visit_q_or_i_array(node) - return + return capture_output { visit_q_or_i_array(node) } end _, elements = node diff --git a/spec/lib/rufo/formatter_source_specs/array_literal.rb.spec b/spec/lib/rufo/formatter_source_specs/array_literal.rb.spec index 9ff92818..bb51be19 100644 --- a/spec/lib/rufo/formatter_source_specs/array_literal.rb.spec +++ b/spec/lib/rufo/formatter_source_specs/array_literal.rb.spec @@ -328,3 +328,11 @@ x = [{ #~# EXPECTED [[], *a] + +#~# ORIGINAL nested_different_array_types + +[%w[( )]] + +#~# EXPECTED + +[%w[( )]]