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 formatter for the charset .editorconfig option #330

Merged
merged 1 commit into from
Aug 15, 2019

Conversation

JoeRobich
Copy link
Member

charset support was requested by the community and should wrap up our support for the the core .editorconfig options.

@RikkiGibson RikkiGibson self-requested a review August 13, 2019 05:42
var codeEncoding = CharsetFormatter.GetCharset(codeValue);
var expectedEncoding = CharsetFormatter.GetCharset(expectedValue);

var testCode = "class C { }";
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be worthwhile to verify that the encoding really changed the test code contents, perhaps by making the test class name a more complex character, and including a byte[] with the final raw content in the InlineData

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, we will need to make an integration test for that. There wouldn't be any change until the workspace has saved the document changes. Since these tests use AdhocWorkspace that isn't possible because it doesn't implement persistence.

@JoeRobich JoeRobich force-pushed the add-charset-formatter branch from 1e95981 to 87b050c Compare August 14, 2019 16:25
@JoeRobich JoeRobich merged commit 0a22d16 into dotnet:master Aug 15, 2019
@JoeRobich JoeRobich deleted the add-charset-formatter branch March 5, 2021 21:01
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.

2 participants