-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Preserve file encoding in file-based app source files #52055
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
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
92f12aa
Initial plan
Copilot 9b7a4f7
Fix UTF-8 BOM preservation in file-based programs
Copilot 13dd3b6
Address PR feedback: use SourceText.Encoding and stackalloc
Copilot c061b9e
Simplify encoding detection using SourceText auto-detection
Copilot 643569f
Add test for non-UTF-8 encoding preservation
Copilot 07074b6
Use collection expressions in asserts
jjonescz 1f177e9
Verify complete file content in PreservesNoBomEncoding test
Copilot a4b9dff
Fix test to use explicit newlines instead of raw string literal
Copilot File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -517,6 +517,95 @@ public void RemoveMultiple() | |
| """)); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Verifies that files without UTF-8 BOM don't get one added when saved. | ||
| /// This is critical for shebang (#!) scripts on Unix-like systems. | ||
| /// <see href="https://github.com/dotnet/sdk/issues/52054"/> | ||
| /// </summary> | ||
| [Fact] | ||
| public void PreservesNoBomEncoding() | ||
| { | ||
| var testInstance = _testAssetsManager.CreateTestDirectory(); | ||
| var tempFile = Path.Join(testInstance.Path, "test.cs"); | ||
|
|
||
| // Create a file without BOM | ||
| var content = "#!/usr/bin/env dotnet run\nConsole.WriteLine();"; | ||
| File.WriteAllText(tempFile, content, new UTF8Encoding(encoderShouldEmitUTF8Identifier: false)); | ||
|
|
||
| // Load, modify, and save | ||
| var sourceFile = SourceFile.Load(tempFile); | ||
| var editor = FileBasedAppSourceEditor.Load(sourceFile); | ||
| editor.Add(new CSharpDirective.Package(default) { Name = "MyPackage", Version = "1.0.0" }); | ||
| editor.SourceFile.Save(); | ||
|
|
||
| // Verify no BOM was added | ||
| var bytes = File.ReadAllBytes(tempFile); | ||
| Assert.True(bytes is not [0xEF, 0xBB, 0xBF, ..], | ||
| "File should not have UTF-8 BOM"); | ||
|
|
||
| // Verify the complete file content is correct | ||
| var savedContent = File.ReadAllText(tempFile); | ||
| var expectedContent = "#!/usr/bin/env dotnet run\n\n#:package [email protected]\n\nConsole.WriteLine();"; | ||
| Assert.Equal(expectedContent, savedContent); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Verifies that files with UTF-8 BOM preserve it when saved. | ||
| /// <see href="https://github.com/dotnet/sdk/issues/52054"/> | ||
| /// </summary> | ||
| [Fact] | ||
| public void PreservesBomEncoding() | ||
| { | ||
| var testInstance = _testAssetsManager.CreateTestDirectory(); | ||
| var tempFile = Path.Join(testInstance.Path, "test.cs"); | ||
|
|
||
| // Create a file with BOM | ||
| var content = "Console.WriteLine();"; | ||
| File.WriteAllText(tempFile, content, new UTF8Encoding(encoderShouldEmitUTF8Identifier: true)); | ||
|
|
||
| // Load, modify, and save | ||
| var sourceFile = SourceFile.Load(tempFile); | ||
| var editor = FileBasedAppSourceEditor.Load(sourceFile); | ||
| editor.Add(new CSharpDirective.Package(default) { Name = "MyPackage", Version = "1.0.0" }); | ||
| editor.SourceFile.Save(); | ||
|
|
||
| // Verify BOM is still present | ||
| var bytes = File.ReadAllBytes(tempFile); | ||
| Assert.True(bytes is [0xEF, 0xBB, 0xBF, ..], | ||
| "File should have UTF-8 BOM"); | ||
| } | ||
jjonescz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| /// <summary> | ||
| /// Verifies that files with non-UTF-8 encodings (like UTF-16) preserve their encoding when saved. | ||
| /// <see href="https://github.com/dotnet/sdk/issues/52054"/> | ||
| /// </summary> | ||
| [Fact] | ||
| public void PreservesNonUtf8Encoding() | ||
| { | ||
| var testInstance = _testAssetsManager.CreateTestDirectory(); | ||
| var tempFile = Path.Join(testInstance.Path, "test.cs"); | ||
|
|
||
| // Create a file with UTF-16 encoding (includes BOM by default) | ||
| var content = "Console.WriteLine(\"UTF-16 test\");"; | ||
| File.WriteAllText(tempFile, content, Encoding.Unicode); | ||
|
|
||
| // Load, modify, and save | ||
| var sourceFile = SourceFile.Load(tempFile); | ||
| var editor = FileBasedAppSourceEditor.Load(sourceFile); | ||
| editor.Add(new CSharpDirective.Package(default) { Name = "MyPackage", Version = "1.0.0" }); | ||
| editor.SourceFile.Save(); | ||
|
|
||
| // Verify UTF-16 BOM is still present (0xFF 0xFE for UTF-16 LE) | ||
| var bytes = File.ReadAllBytes(tempFile); | ||
| Assert.True(bytes is [0xFF, 0xFE, ..], | ||
| "File should have UTF-16 LE BOM"); | ||
|
|
||
| // Verify content is still readable as UTF-16 | ||
| var savedContent = File.ReadAllText(tempFile, Encoding.Unicode); | ||
| Assert.Contains("#:package [email protected]", savedContent); | ||
| Assert.Contains("Console.WriteLine", savedContent); | ||
| } | ||
|
|
||
| private void Verify( | ||
| string input, | ||
| params ReadOnlySpan<(Action<FileBasedAppSourceEditor> action, string expectedOutput)> verify) | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.