Merge pull request #812 from Shopify/vs/improve_pipe_completion_on_manual_insert

Improve pipe completion if the user adds an accidental extra pipe

Author: vinistock

lib/ruby_lsp/requests/on_type_formatting.rb

@@ -62,9 +62,34 @@ def run
 
       sig { void }
       def handle_pipe
-        return unless /((?<=do)|(?<={))\s+\|/.match?(@previous_line)
+        current_line = @lines[@position[:line]]
+        return unless /((?<=do)|(?<={))\s+\|/.match?(current_line)
+
+        line = T.must(current_line)
+
+        # If the current character is a pipe and both previous ones are pipes too, then we autocompleted a pipe and the
+        # user inserted a third one. In this case, we need to avoid adding a fourth and remove the previous one
+        if line[@position[:character] - 2] == "|" &&
+            line[@position[:character] - 1] == "|" &&
+            line[@position[:character]] == "|"
+
+          @edits << Interface::TextEdit.new(
+            range: Interface::Range.new(
+              start: Interface::Position.new(
+                line: @position[:line],
+                character: @position[:character],
+              ),
+              end: Interface::Position.new(
+                line: @position[:line],
+                character: @position[:character] + 1,
+              ),
+            ),
+            new_text: "",
+          )
+        else
+          add_edit_with_text("|")
+        end
 
-        add_edit_with_text("|")
         move_cursor_to(@position[:line], @position[:character])
       end
 

test/requests/on_type_formatting_test.rb

@@ -68,14 +68,14 @@ def test_adding_missing_pipe
     )
     document.parse
 
-    edits = RubyLsp::Requests::OnTypeFormatting.new(document, { line: 0, character: 11 }, "|").run
+    edits = RubyLsp::Requests::OnTypeFormatting.new(document, { line: 0, character: 12 }, "|").run
     expected_edits = [
       {
-        range: { start: { line: 0, character: 11 }, end: { line: 0, character: 11 } },
+        range: { start: { line: 0, character: 12 }, end: { line: 0, character: 12 } },
         newText: "|",
       },
       {
-        range: { start: { line: 0, character: 11 }, end: { line: 0, character: 11 } },
+        range: { start: { line: 0, character: 12 }, end: { line: 0, character: 12 } },
         newText: "$0",
       },
     ]
@@ -94,10 +94,66 @@ def test_pipe_is_not_added_in_regular_or_pipe
     )
     document.parse
 
-    edits = RubyLsp::Requests::OnTypeFormatting.new(document, { line: 0, character: 11 }, "|").run
+    edits = RubyLsp::Requests::OnTypeFormatting.new(document, { line: 0, character: 2 }, "|").run
     assert_empty(T.must(edits))
   end
 
+  def test_pipe_is_removed_if_user_adds_manually_after_completion
+    document = RubyLsp::Document.new(source: +"", version: 1, uri: "file:///fake.rb")
+
+    document.push_edits(
+      [{
+        range: { start: { line: 0, character: 0 }, end: { line: 0, character: 0 } },
+        text: "[].each do |",
+      }],
+      version: 2,
+    )
+    document.parse
+
+    document.push_edits(
+      [{
+        range: { start: { line: 0, character: 12 }, end: { line: 0, character: 12 } },
+        text: "|",
+      }],
+      version: 3,
+    )
+    edits = RubyLsp::Requests::OnTypeFormatting.new(document, { line: 0, character: 12 }, "|").run
+    expected_edits = [
+      {
+        range: { start: { line: 0, character: 12 }, end: { line: 0, character: 12 } },
+        newText: "|",
+      },
+      {
+        range: { start: { line: 0, character: 12 }, end: { line: 0, character: 12 } },
+        newText: "$0",
+      },
+    ]
+    assert_equal(expected_edits.to_json, T.must(edits).to_json)
+    assert_equal("[].each do ||", document.source)
+
+    # Push the third pipe manually after the completion happened
+    document.push_edits(
+      [{
+        range: { start: { line: 0, character: 13 }, end: { line: 0, character: 13 } },
+        text: "|",
+      }],
+      version: 3,
+    )
+
+    edits = RubyLsp::Requests::OnTypeFormatting.new(document, { line: 0, character: 13 }, "|").run
+    expected_edits = [
+      {
+        range: { start: { line: 0, character: 13 }, end: { line: 0, character: 14 } },
+        newText: "",
+      },
+      {
+        range: { start: { line: 0, character: 13 }, end: { line: 0, character: 13 } },
+        newText: "$0",
+      },
+    ]
+    assert_equal(expected_edits.to_json, T.must(edits).to_json)
+  end
+
   def test_comment_continuation
     document = RubyLsp::Document.new(source: +"", version: 1, uri: "file:///fake.rb")