Skip to content

Conversation

@sharwell
Copy link
Contributor

@sharwell sharwell commented Feb 23, 2023

Closes #18065
Closes #45895
Closes #67244
Closes #67245

@sharwell sharwell marked this pull request as ready for review February 27, 2023 19:29
@sharwell sharwell requested a review from a team as a code owner February 27, 2023 19:29
@CyrusNajmabadi
Copy link
Member

I don't see anything about this PR taht prevents #67073 from going in. All that PR does is extract the same logic that rename has today into a reusable class, instead of embedding it into the inline-rename type.

await TestServices.EditorVerifier.TextEqualsAsync(@"
Imports System

Public Class CustomAttribute$$
Copy link
Member

@genlu genlu Mar 14, 2023

Choose a reason for hiding this comment

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

Suggested change
Public Class CustomAttribute$$
Public Class CustomA$$ttribute

This is what the actual result looks like in the failure. I manually tried inline rename with 17.6, the cursor position after rename is consistent with test result. I think we should adjust the baseline here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tested Version 17.6.0 Preview 3.0 [33505.110.main] both with and without this change, and so far I have not been able to reproduce a caret position anywhere except at the end of the line.

Copy link
Member

@genlu genlu Mar 15, 2023

Choose a reason for hiding this comment

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

This is what I got in 17.6p3
rename

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a different test. The one here is only the behavior for attributes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I modified the test to support both caret placement outcomes. It's not clear how/why the caret gets placed at the end of a rename operation so I wasn't sure how to start debugging this.

@sharwell sharwell merged commit 2b378ff into dotnet:main Mar 15, 2023
@sharwell sharwell deleted the port-tests branch March 15, 2023 15:33
@ghost ghost added this to the Next milestone Mar 15, 2023
@allisonchou allisonchou modified the milestones: Next, 17.6 P3 Mar 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

4 participants