Skip to content

split ruby blocks correctly when ; is inside string literal #199

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
AndrewRadev merged 5 commits into from
Mar 21, 2023
Merged

split ruby blocks correctly when ; is inside string literal #199

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6e2756a
split ruby blocks correctly when ; is inside string literal
rrhubenov Feb 24, 2023
6bd9fcc
tests for the 3 types of ruby string literals
rrhubenov Feb 28, 2023
2949ec6
replace body and loop over ';' skipping rubyString syntax group
rrhubenov Feb 28, 2023
6509663
don't wrap around buffer when searching replacement
rrhubenov Feb 28, 2023
64b9774
remove unnecessary search
rrhubenov Mar 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions autoload/sj/ruby.vim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,14 +358,19 @@ function! sj#ruby#SplitBlock()
if search('\S\%#', 'Wbn')
let multiline_block = ' '.multiline_block
endif

let body = join(split(body, '\s*;\s*'), "\n")

let replacement = substitute(body, '^'.pattern.'$', multiline_block, '')

" remove leftover whitespace
let replacement = substitute(replacement, '\s*\n', '\n', 'g')

call sj#ReplaceMotion('Va{', replacement)

normal! j0
Copy link
Contributor Author

@rrhubenov rrhubenov Feb 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AndrewRadev
I implemented the second method you described.
But since search(replacement, 'cW') jumps one line above the body of the "do; end" block, I had to
use "normal! j0" to move to the beginning of the correct line.
It seems to me that moving 1 line lower is kind of dirty, but I'm not sure.

I could use sj#GetMotion('Vi{') but I suspect there might be a cleaner way.
Do you have any suggestions :)?

Copy link
Owner

@AndrewRadev AndrewRadev Mar 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem in j0, you'll see me do this kind of thing elsewhere as well :). Nothing dirty about using the basic motions.

A practical problem, though, is searching for the replacement. What if the replacement has special characters, like *? You can try it with foo * bar, which as a pattern is "foo, followed by spaces, followed by bar", which won't actually match the string :). It's possible to search for a literal string by prefixing it with \V, but it feels unnecessary. Why the search? Where are you trying to position the cursor? Wouldn't just j0 be enough to get you in the right place? At least when I'm testing the code out, it seems like removing the search still works fine. Tests are passing too.

Copy link
Contributor Author

@rrhubenov rrhubenov Mar 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, great!
I wasn't sure where ReplaceMotion would leave the cursor.

I've left only j0. Less code, less bugs :)

while sj#SearchSkip(';', sj#SkipSyntax(['rubyString']), 'W', line('.')) > 0
call execute("normal! r\<cr>")
endwhile

return 1
endfunction

Expand Down
80 changes: 80 additions & 0 deletions spec/plugin/ruby_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -955,6 +955,86 @@ module Bar
EOF
end

it "doesn't get confused by ; when joining" do
set_file_contents <<~EOF
foo do
example1
example2
end
EOF

vim.search 'do'
join

assert_file_contents <<~EOF
foo { example1; example2 }
EOF
end

it "doesn't get confused by ; when splitting" do
set_file_contents <<~EOF
foo { example1; example2 }
EOF

vim.search 'example1;'
split

assert_file_contents <<~EOF
foo do
example1
example2
end
EOF
end

it "doesn't get confused when ; is in a string literal(single quotes) when splitting" do
set_file_contents <<~EOF
foo { example1; 'some string ; literal' }
EOF

vim.search 'example1;'
split

assert_file_contents <<~EOF
foo do
example1
'some string ; literal'
end
EOF
end

it "doesn't get confused when ; is in a string literal(double quotes) when splitting" do
set_file_contents <<~EOF
foo { example1; "some string ; literal" }
EOF

vim.search 'example1;'
split

assert_file_contents <<~EOF
foo do
example1
"some string ; literal"
end
EOF
end

it "doesn't get confused when ; is in a string literal(percent strings) when splitting" do
set_file_contents <<~EOF
foo { example1; %(some string ; literal) }
EOF

vim.search 'example1;'
split

assert_file_contents <<~EOF
foo do
example1
%(some string ; literal)
end
EOF
end

it "migrates inline comments when joining" do
set_file_contents <<~EOF
foo do
Expand Down