Skip to content

Fix invert if for cases where the if block is empty#54423

Merged
ryzngard merged 7 commits intodotnet:mainfrom
ryzngard:issues/43224_invert_if_empty
Jul 1, 2021
Merged

Fix invert if for cases where the if block is empty#54423
ryzngard merged 7 commits intodotnet:mainfrom
ryzngard:issues/43224_invert_if_empty

Conversation

@ryzngard
Copy link
Copy Markdown
Contributor

Fixes #43224

@ryzngard ryzngard requested a review from a team as a code owner June 27, 2021 01:19
@ghost ghost added the Area-IDE label Jun 27, 2021
{
await TestInRegularAndScriptAsync(
@"class C { void M(string s){ [||]if (s == ""a""){}else{ s = ""b""}}}",
@"class C { void M(string s){ if (s != ""a""){ s = ""b""}}}");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

tbh, this seems a bit strange. but i guess it's fine :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

which part?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

taht we are throwing away the empty block. it feels like if hte user asks to invert, we should just invert. another feature could potentially say: this block is useless

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

oh...gotcha. I'm going to leave for now because it was my gut that it's how it should work. That said, I'm not really opposed to leaving the empty block if we find that people don't like it being removed

end class",
"class C
sub M(x as String)
If x IsNot ""a"" Then
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

it's a bit strange that = got replaced withIsNot instead of <>.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I didn't change the logic here... maybe worth a different bug?

Copy link
Copy Markdown
Contributor

@CyrusNajmabadi CyrusNajmabadi left a comment

Choose a reason for hiding this comment

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

Some small pieces of feedback.

@ryzngard ryzngard requested a review from CyrusNajmabadi July 1, 2021 19:21
@ryzngard ryzngard enabled auto-merge July 1, 2021 20:44
@ryzngard ryzngard merged commit dc21aa9 into dotnet:main Jul 1, 2021
@ghost ghost added this to the Next milestone Jul 1, 2021
@allisonchou allisonchou modified the milestones: Next, 17.0.P3 Jul 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

When inverting If with code only in Else section, The code moves to the primary area and is left in the else

4 participants