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

Fix accidental dedent for in and when dedent in Ruby comments #206132

Merged
merged 1 commit into from Feb 23, 2024

Conversation

vinistock
Copy link
Contributor

I was testing the changes from #205397 in Insiders and I noticed that the code was incorrectly being dedented when typing in or when in comments.

We got the parenthesis balance incorrect in the regex. The regex has three outer groups for an or condition and we closed the group incorrectly, which made the last part of the regex skip the \s* part (which verifies that only blank spaces are in the front of the string).

This PR fixes the bug by properly balancing the parenthesis. The tests was also missing the ^ anchor, which I added to match the language configuration.

cc @rebornix

@rebornix
Copy link
Member

@vinistock thanks for the quick fix! Does it only affect comments? We are in endgame mode so I'm trying to understand if it's worth a candidate or if we should pull back the previous change and merge together with this one.

@VSCodeTriageBot VSCodeTriageBot added this to the March 2024 milestone Feb 23, 2024
@rebornix rebornix merged commit 250466a into microsoft:main Feb 23, 2024
6 checks passed
@vinistock
Copy link
Contributor Author

@rebornix because of the unbalanced parenthesis, it ignores the \s* part, meaning it matches any in|when words - not just comments. Even if you start writing a method call like within and put a space, it will incorrectly dedent.

I don't know what the endgame policies are, but in my opinion it's definitely worth either pulling back the previous change or including this one.

@vinistock vinistock deleted the vs/fix_in_dedent_in_comments branch February 24, 2024 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants