Skip to content

Commit

Permalink
Fix small issues with end insertion (#940)
Browse files Browse the repository at this point in the history
* Remove extra space after end insertion

This commit ensures there is no extra space added after the cursor when
on type formatting inserts an `end` keyword. For some reason, if we try
to insert the newline character and the `end` keyword via a single edit
without the extra space, `applyEdit` [1] strips the indentation and the
cursor ends up at the beginning of the middle line. Inserting the
newline via a separate edit doesn't seem to have this problem.

[1]: https://github.com/Shopify/vscode-ruby-lsp/blob/74e45e754aac281a22d5627c22a1a2a3e73d8b33/src/client.ts#L160

* Fix cusor position when breaking after keyword

If the user breaks just after the keyword, this commit ensures the
cursor is placed back where the break happened. This is currently broken
in that the cursor will end up 1 character into the next line. There was
some discussion [1] about whether the cursor should remain where it
would normally be if the end insertion didn't happen, but it appears
moving it back to where the break happened is the intended behavior.

[1]: #462 (comment)

* Add newline when breaking after keyword

This commit ensures that when breaking after the keyword, the `end` is
inserted with a newline, rather than replacing an existing empty line.
  • Loading branch information
thomasmarshall authored and andyw8 committed Sep 5, 2023
1 parent d837e23 commit bec7672
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 5 deletions.
7 changes: 4 additions & 3 deletions lib/ruby_lsp/requests/on_type_formatting.rb
Original file line number Diff line number Diff line change
Expand Up @@ -112,11 +112,12 @@ def handle_statement_end
next_line = @lines[@position[:line] + 1]

if current_line.nil? || current_line.strip.empty?
add_edit_with_text(" \n#{indents}end")
add_edit_with_text("\n")
add_edit_with_text("#{indents}end")
move_cursor_to(@position[:line], @indentation + 2)
elsif next_line.nil? || next_line.strip.empty?
add_edit_with_text("#{indents}end", { line: @position[:line] + 1, character: @position[:character] })
move_cursor_to(@position[:line], @indentation + 3)
add_edit_with_text("#{indents}end\n", { line: @position[:line] + 1, character: @position[:character] })
move_cursor_to(@position[:line] - 1, @indentation + @previous_line.size + 1)
end
end

Expand Down
39 changes: 37 additions & 2 deletions test/requests/on_type_formatting_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,11 @@ def test_adding_missing_ends
expected_edits = [
{
range: { start: { line: 1, character: 2 }, end: { line: 1, character: 2 } },
newText: " \nend",
newText: "\n",
},
{
range: { start: { line: 1, character: 2 }, end: { line: 1, character: 2 } },
newText: "end",
},
{
range: { start: { line: 1, character: 2 }, end: { line: 1, character: 2 } },
Expand Down Expand Up @@ -302,7 +306,11 @@ def test_breaking_line_between_keyword_and_more_content
expected_edits = [
{
range: { start: { line: 1, character: 2 }, end: { line: 1, character: 2 } },
newText: " \nend",
newText: "\n",
},
{
range: { start: { line: 1, character: 2 }, end: { line: 1, character: 2 } },
newText: "end",
},
{
range: { start: { line: 1, character: 2 }, end: { line: 1, character: 2 } },
Expand All @@ -328,4 +336,31 @@ def test_breaking_line_between_keyword_when_there_is_content_on_the_next_line
edits = RubyLsp::Requests::OnTypeFormatting.new(document, { line: 0, character: 2 }, "\n").run
assert_empty(edits)
end

def test_breaking_line_immediately_after_keyword
document = RubyLsp::Document.new(source: +"", version: 1, uri: URI("file:///fake.rb"))

document.push_edits(
[{
range: { start: { line: 0, character: 0 }, end: { line: 0, character: 0 } },
text: " def\nfoo",
}],
version: 2,
)
document.parse

edits = RubyLsp::Requests::OnTypeFormatting.new(document, { line: 1, character: 2 }, "\n").run
expected_edits = [
{
range: { start: { line: 2, character: 2 }, end: { line: 2, character: 2 } },
newText: " end\n",
},
{
range: { start: { line: 0, character: 6 }, end: { line: 0, character: 6 } },
newText: "$0",
},
]

assert_equal(expected_edits.to_json, T.must(edits).to_json)
end
end

0 comments on commit bec7672

Please sign in to comment.