Try and make single-call breakables less ocassionally-gross

fables-tales · Feb 22, 2024 · 28c6331 · 28c6331
1 parent b72b757
commit 28c6331
Show file tree

Hide file tree

Showing 7 changed files with 72 additions and 50 deletions.
diff --git a/fixtures/small/block_param_line_length_expected.rb b/fixtures/small/block_param_line_length_expected.rb
@@ -20,5 +20,6 @@
   .quux
   .what_comes_after_quux_idk_aaaahhhhhhh
   .map(&:foo)
-ThisIsAReallyLongClassName::ButSlightShorterWithMoreCalls::ThisIsAReallyLongClassName::ButSlightShorter
-  .new(foo: bar_baz_quuz)
+ThisIsAReallyLongClassName::ButSlightShorterWithMoreCalls::ThisIsAReallyLongClassName::ButSlightShorter.new(
+  foo: bar_baz_quuz
+)
diff --git a/fixtures/small/breakables_over_line_length_expected.rb b/fixtures/small/breakables_over_line_length_expected.rb
@@ -18,7 +18,9 @@
   ReallyLongThing
 ]
 
-if Opus::Foo::Bar::Baz::ReallyLongName::Example::ExampleExampleExampleExampleExampleExampleExampleExample
-    .calls_a_thing(a, b)
+if Opus::Foo::Bar::Baz::ReallyLongName::Example::ExampleExampleExampleExampleExampleExampleExampleExample.calls_a_thing(
+    a,
+    b
+  )
   puts("")
 end
diff --git a/fixtures/small/heredoc_method_call_expected.rb b/fixtures/small/heredoc_method_call_expected.rb
@@ -1,35 +1,34 @@
 class William::Carlos::Williams
-  landscape_with_the_fall_of_icarus = T
-    .let(
-      new(
-        <<~LANDSCAPE
-          According to Brueghel
-          when Icarus fell
-          it was spring
-
-          a farmer was ploughing
-          his field
-          the whole pageantry
-
-          of the year was
-          awake tingling
-          with itself
-
-          sweating in the sun
-          that melted
-          the wings' wax
-
-          unsignificantly
-          off the coast
-          there was
-
-          a splash quite unnoticed
-          this was
-          Icarus drowning
-        LANDSCAPE
-      ),
-      Williams
-    )
+  landscape_with_the_fall_of_icarus = T.let(
+    new(
+      <<~LANDSCAPE
+        According to Brueghel
+        when Icarus fell
+        it was spring
+
+        a farmer was ploughing
+        his field
+        the whole pageantry
+
+        of the year was
+        awake tingling
+        with itself
+
+        sweating in the sun
+        that melted
+        the wings' wax
+
+        unsignificantly
+        off the coast
+        there was
+
+        a splash quite unnoticed
+        this was
+        Icarus drowning
+      LANDSCAPE
+    ),
+    Williams
+  )
 end
 
 optp

diff --git a/fixtures/small/long_blockvar_expected.rb b/fixtures/small/long_blockvar_expected.rb
@@ -1,12 +1,13 @@
-things.each do |
-    omg:,
-    really:,
-    dang:,
-    long:,
-    blockvar:,
-    that_is_so_long_if_you_write_this:,
-    you_should_refactor:,
-    like_really_this_is_so_long:
-  |
-  do_things!
-end
+things
+  .each do |
+      omg:,
+      really:,
+      dang:,
+      long:,
+      blockvar:,
+      that_is_so_long_if_you_write_this:,
+      you_should_refactor:,
+      like_really_this_is_so_long:
+    |
+    do_things!
+  end
diff --git a/fixtures/small/multiline_chain_in_block_actual.rb b/fixtures/small/multiline_chain_in_block_actual.rb
@@ -6,3 +6,8 @@
 def ajax_get(route)
   super
 end
+
+class Foo
+  sig {override.returns(T::Array[T.class_of(Some::Really::Long::Name::ThatshouldprobablybealisedbutisntbecauseThis::IsATestStub)])}
+  def example = begin; end
+end
diff --git a/fixtures/small/multiline_chain_in_block_expected.rb b/fixtures/small/multiline_chain_in_block_expected.rb
@@ -7,3 +7,13 @@
 def ajax_get(route)
   super
 end
+
+class Foo
+  sig {
+    override.returns(
+      T::Array[T.class_of(Some::Really::Long::Name::ThatshouldprobablybealisedbutisntbecauseThis::IsATestStub)]
+    )
+  }
+  def example = begin
+  end
+end
diff --git a/librubyfmt/src/render_targets.rs b/librubyfmt/src/render_targets.rs
@@ -304,10 +304,14 @@ impl AbstractTokenTarget for BreakableCallChainEntry {
         {
             tokens.pop();
         }
-        // If the last breakable is multiline (and not a block), ignore it. The user likely
-        // intentionally chose a line break strategy, so try our best to respect it
+        let call_count = tokens.iter().filter(|t| matches!(t, AbstractLineToken::ConcreteLineToken(ConcreteLineToken::Dot | ConcreteLineToken::LonelyOperator))).count();
+        // If the last breakable is multiline (and not a block/block params), ignore it. The user likely
+        // intentionally chose a line break strategy, so try our best to respect it.
+        //
+        // However, if there's only one item in the chain, try our best to leave that in place.
+        // `foo\n.bar` is always a little awkward.
         if let Some(AbstractLineToken::BreakableEntry(be)) = tokens.last() {
-            if be.is_multiline() && be.delims != BreakableDelims::for_brace_block() {
+            if (call_count == 1 || be.is_multiline()) && be.delims != BreakableDelims::for_brace_block() && be.delims != BreakableDelims::for_block_params() {
                 tokens.pop();
             }
         }