Remove need for visitor

Author: kddnewton

bin/profile

@@ -14,3 +14,6 @@ StackProf.run(mode: :cpu, out: "tmp/profile.dump", raw: true) do
 end
 
 GC.enable
+
+`bundle exec stackprof --d3-flamegraph tmp/profile.dump > tmp/flamegraph.html`
+puts "open tmp/flamegraph.html"

lib/syntax_tree.rb

@@ -2303,39 +2303,48 @@ def on_binary(left, operator, right)
     )
   end
 
-  # This is a visitor that can be passed to PrettyPrint.visit that will
-  # propagate BreakParent nodes all of the way up the tree. When a BreakParent
-  # is encountered, it will break the surrounding group, and then that group
-  # will break its parent, and so on.
-  class RemoveBreaksVisitor
-    def on_enter(doc)
-      case doc
-      when PrettyPrint::Indent, PrettyPrint::Align, PrettyPrint::Group
-        doc.contents.map! { |child| remove_breaks(child) }
-      when PrettyPrint::IfBreak
-        doc.flat_contents.map! { |child| remove_breaks(child) }
-      when PrettyPrint::LineSuffix
-        return false
-      end
+  # This module will remove any breakables from the list of contents so that no
+  # newlines are present in the output.
+  module RemoveBreaks
+    class << self
+      def call(doc)
+        marker = Object.new
+        stack = [doc]
+
+        while stack.any?
+          doc = stack.pop
+
+          if doc == marker
+            stack.pop
+            next
+          end
 
-      true
-    end
+          stack += [doc, marker]
 
-    def on_exit(doc)
-    end
+          case doc
+          when PrettyPrint::Align, PrettyPrint::Indent, PrettyPrint::Group
+            doc.contents.map! { |child| remove_breaks(child) }
+            stack += doc.contents.reverse
+          when PrettyPrint::IfBreak
+            doc.flat_contents.map! { |child| remove_breaks(child) }
+            stack += doc.flat_contents.reverse
+          end
+        end
+      end
 
-    private
+      private
 
-    def remove_breaks(doc)
-      case doc
-      when PrettyPrint::Breakable
-        text = PrettyPrint::Text.new
-        text.add(object: doc.force? ? "; " : doc.separator, width: doc.width)
-        text
-      when PrettyPrint::IfBreak
-        PrettyPrint::Align.new(indent: 0, contents: doc.flat_contents)
-      else
-        doc
+      def remove_breaks(doc)
+        case doc
+        when PrettyPrint::Breakable
+          text = PrettyPrint::Text.new
+          text.add(object: doc.force? ? "; " : doc.separator, width: doc.width)
+          text
+        when PrettyPrint::IfBreak
+          PrettyPrint::Align.new(indent: 0, contents: doc.flat_contents)
+        else
+          doc
+        end
       end
     end
   end
@@ -2374,7 +2383,7 @@ def child_nodes
     def format(q)
       q.group(0, "|", "|") do
         doc = q.format(params)
-        PrettyPrint.visit(doc, RemoveBreaksVisitor.new)
+        RemoveBreaks.call(doc)
 
         if locals.any?
           q.text("; ")
@@ -10647,7 +10656,7 @@ def format(q)
         # assume that's the way the developer wanted this expression
         # represented.
         doc = q.group(0, '#{', "}") { q.format(statements) }
-        PrettyPrint.visit(doc, RemoveBreaksVisitor.new)
+        RemoveBreaks.call(doc)
       else
         q.group do
           q.text('#{')

lib/syntax_tree/prettyprint.rb

@@ -590,37 +590,6 @@ def self.singleline_format(output = ''.dup, maxwidth = nil, newline = nil, gensp
     output
   end
 
-  # This method provides a way to walk through the print tree with a specified
-  # +visitor+ object. +visitor+ should respond to both #on_enter(doc) and
-  # #on_exit(doc).
-  def self.visit(doc, visitor)
-    marker = Object.new
-    stack = [doc]
-
-    while stack.any?
-      doc = stack.pop
-
-      if doc == marker
-        visitor.on_exit(stack.pop)
-        next
-      end
-
-      stack += [doc, marker]
-
-      if visitor.on_enter(doc)
-        case doc
-        when Array
-          doc.reverse_each { |part| stack << part }
-        when IfBreak
-          stack << doc.break_contents if doc.break_contents
-          stack << doc.flat_contents if doc.flat_contents
-        when Align, Indent, Group, LineSuffix
-          stack << doc.contents
-        end
-      end
-    end
-  end
-
   # The output object. It represents the final destination of the contents of
   # the print tree. It should respond to <<.
   #
@@ -717,10 +686,8 @@ def current_group
   # Flushes all of the generated print tree onto the output buffer, then clears
   # the generated tree from memory.
   def flush
-    # First, ensure that we've propagated all of the necessary break-parent
-    # nodes throughout the tree.
+    # First, get the root group, since we placed one at the top to begin with.
     doc = groups.first
-    propagate_breaks(doc)
 
     # This represents how far along the current line we are. It gets reset
     # back to 0 when we encounter a newline.
@@ -879,6 +846,11 @@ def break_parent
     doc = BreakParent.new
     target << doc
 
+    groups.reverse_each do |group|
+      break if group.break?
+      group.break
+    end
+
     doc
   end
 
@@ -1101,37 +1073,6 @@ def fits?(next_command, rest_commands, remaining)
     false
   end
 
-  # This will walk the doc tree and propagate BreakParent nodes all of the way
-  # up the tree. When a BreakParent is encountered, it will break the
-  # surrounding group, and then that group will break its parent, and so on.
-  def propagate_breaks(doc)
-    marker = Object.new
-    stack = [doc]
-    groups = []
-
-    while doc = stack.pop
-      if doc == marker
-        groups.last&.break if stack.pop.is_a?(Group) && groups.pop.break?
-      else
-        stack += [doc, marker]
-        enter = false
-
-        case doc
-        when BreakParent
-          groups.last&.break
-        when Group
-          groups << doc
-          stack += doc.contents.reverse
-        when IfBreak
-          stack += doc.break_contents.reverse
-          stack += doc.flat_contents.reverse
-        when Align, Indent, LineSuffix
-          stack += doc.contents.reverse
-        end
-      end
-    end
-  end
-
   # Resets the group stack and target array so that this pretty printer object
   # can continue to be used before calling flush again if desired.
   def reset