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

Add support for smart string/copy paste when the destination is a raw string literal. #61177

Merged
merged 32 commits into from
May 9, 2022

Conversation

CyrusNajmabadi
Copy link
Member

This includes properly fixing up the delimiters (adding $ and " as necessayr) as well as handling indentation and also escaping internal braces on interpolations as necessary.

Lots of tests added. Several hundred more to go there.

@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner May 8, 2022 01:24
}

return ImmutableArray.Create(new TextChange(_selectionBeforePaste.Span.ToTextSpan(), builder.ToString()));
}
Copy link
Member Author

Choose a reason for hiding this comment

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

this method called out to two methods that were almost identical in how they behaved. so i merged those two methods into one, and then tweaked it in the one place they differ in behavior.

// At this point, we will now have the information necessary to actually insert the content and do things
// like give interpolations the proper number of braces for the final string we're making.

var dummyContentEdit = GetContentEditForRawString(insertInterpolations: false, dollarSignCount: -1, indentationWhitespace: "");
Copy link
Member Author

Choose a reason for hiding this comment

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

this method is very long, but is described by the above comment. i intend to break it apart into pieces to make it clearer as to what happens.

}
}

private TextChange GetContentEditForRawString(
Copy link
Member Author

Choose a reason for hiding this comment

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

ignore the diff in this mehtod. it comes from the fact that i merged two similar methods into one (talked above above), but then added a method for the raw case. the diff thinkgs the raw-method shoudl diff against one of the prior methods.

@CyrusNajmabadi
Copy link
Member Author

@davidwengier let me know what you think, and if there are ways to make things clearer.

Copy link
Contributor

@davidwengier davidwengier left a comment

Choose a reason for hiding this comment

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

Logic makes sense, though one thing that isn't clear (and I haven't looked through the tests) but is there coverage for the scenario where the newly pasted interpolations need to have braces added? eg, if pasting from a $""" to a $$$""" or something.

Though I imagine this stuff gets extremely complicated so its probably fair enough if some things just aren't supported.

@CyrusNajmabadi
Copy link
Member Author

but is there coverage for the scenario where the newly pasted interpolations need to have braces added? eg, if pasting from a $""" to a $$$""" or something.

Yup. Test for that and tests for ensuring that if we need to change the number of '$' that we also update existing interpolations.

@CyrusNajmabadi CyrusNajmabadi enabled auto-merge May 9, 2022 02:16
@CyrusNajmabadi CyrusNajmabadi merged commit 40b9280 into dotnet:main May 9, 2022
@ghost ghost added this to the Next milestone May 9, 2022
@CyrusNajmabadi CyrusNajmabadi deleted the stringCopyPaste5 branch May 9, 2022 14:08
@Cosifne Cosifne modified the milestones: Next, 17.3 P2 May 31, 2022
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.

3 participants