Skip to content
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.

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

feat: implement remove newlines in brackets before toggle block to on… #3005

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Hungle2911
Copy link
Contributor

Motivation

Close #2968
The current block conversion from do..end to brackets fails when handling complex data structures like Hashes or Arrays. Simply converting line breaks to semicolons doesn't maintain code validity in these cases.

Implementation

  • Added remove_newlines_in_brackets method to properly handle newlines within Hash/Array literals

Automated Tests

  1. test_toggle_block_do_end_to_brackets: Verifies basic block conversion functionality
  2. test_toggle_block_do_end_with_hash_to_brackets: Ensures proper handling of Hash literals within blocks

@Hungle2911 Hungle2911 requested a review from a team as a code owner January 2, 2025 23:31
@@ -272,6 +272,33 @@ def create_text_edit(range, new_text)
)
end

sig { params(str: String).returns(String) }
def remove_newlines_in_brackets(str)
Copy link
Member

Choose a reason for hiding this comment

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

This approach is essentially re-parsing the string that was already parsed by Prism to remove the newlines. I'm concerned that this will be quite error prone, since parsing Ruby is challenging.

For example, will the code work properly in a case like this?

[].each do |something|
  # Brackets that don't represent an array mixed with arrays
  a[:hello] = [
    something[1],
  ]
end

If we could find a way to use the already parsed nodes from Prism to rebuild the block's body, I think that would be more robust. Maybe we need to go through the nodes inside the body and transform them into single line one by one, with different rules for hash and array nodes so that we can produce valid code.

@vinistock vinistock added bugfix This PR will fix an existing bug server This pull request should be included in the server gem's release notes labels Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This PR will fix an existing bug server This pull request should be included in the server gem's release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Switch block style refactoring needs improvement
2 participants