Skip to content

Default to an Encoding without BOM when BOM marker was not found #42

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 3 commits into from
Oct 6, 2024

Conversation

mdonatas
Copy link

@mdonatas mdonatas commented Oct 5, 2024

OpenStream methods attempts detecting encoding by checking for BOM at the beginning of the stream. When that fails a call to AutoDetect is made. Given that AutoDetect is entered knowing that BOM was not found, an Encoding of StreamReader that it uses should be without BOM.
This doesn't impact reading the file but it does impact the receiving/consuming end of a StreamReader where a code might check what encoding is set on a given StreamReader to use the same encoding for saving edited file which would lead to incorrectly adding BOM.

StreamReader constructor defaults to Encoding.UTF8 which has BOM thus encoding used to create a file doesn't match encoding set on the StreamReader

public StreamReader(Stream stream, bool detectEncodingFromByteOrderMarks)
    : this(stream, Encoding.UTF8, detectEncodingFromByteOrderMarks, DefaultBufferSize, false)
{
}

@mdonatas
Copy link
Author

mdonatas commented Oct 5, 2024

p.s. this is a part of a fix for gitextensions/gitextensions#11934

Copy link
Member

@mstv mstv left a comment

Choose a reason for hiding this comment

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

LGTM, have not run

Co-authored-by: Igor Velikorossov <RussKie@users.noreply.github.com>
@RussKie RussKie merged commit 4ea04d2 into gitextensions:master Oct 6, 2024
2 checks passed
@mdonatas mdonatas deleted the no-bom-when-no-bom branch October 6, 2024 08:00
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.

3 participants