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

fix encoding of NfcProtocol.cs #2055

Merged
merged 1 commit into from
Mar 23, 2023
Merged

fix encoding of NfcProtocol.cs #2055

merged 1 commit into from
Mar 23, 2023

Conversation

jdbruner
Copy link
Contributor

@jdbruner jdbruner commented Mar 18, 2023

Mea culpa.

Card/NfcProtocol.cs was created with the wrong encoding (was utf8, should be utf8bom)

Microsoft Reviewers: Open in CodeFlow

@ghost ghost added the area-device-bindings Device Bindings for audio, sensor, motor, and display hardware that can used with System.Device.Gpio label Mar 18, 2023
@krwq
Copy link
Member

krwq commented Mar 18, 2023

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@krwq
Copy link
Member

krwq commented Mar 18, 2023

@pgrawehr possibly we need to rebase or make some artificial change so that your changes get picked up?

@krwq
Copy link
Member

krwq commented Mar 19, 2023

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@krwq
Copy link
Member

krwq commented Mar 19, 2023

@jdbruner I think no-bom should be the default, if it's not, it's most likely incorrect :-)

@jdbruner
Copy link
Contributor Author

@krwq I didn't notice this earlier, but something changed in my environment and I started getting a build error complaining about it.

Frankly I'd prefer no BOM. However, the .editorconfig does specify UTF-8 with BOM.

@krwq
Copy link
Member

krwq commented Mar 19, 2023

it sounds editor config is setup incorrectly

@pgrawehr
Copy link
Contributor

No, this is on purpose. The bom guarantees correct encoding. We have several source files where the correct encoding is critical, as they contain non-ascii characters. Without the bom marker, it's likely some editors will get it wrong.

@krwq
Copy link
Member

krwq commented Mar 19, 2023

@pgrawehr I think at dotnet/runtime at least we've established no-BOM around 2015 as default because at this point UTF-8 is the standard and everything else is mostly some old relic. We should make sure we pick one and stick with it and AFAIK it's UTF-8 no BOM everywhere.

@pgrawehr
Copy link
Contributor

@krwq At the moment, we have it everywhere, and I did have bad experiences when it was missing, so we shouldn't change it unless we see a reason. But let's discuss in the next call.

@krwq
Copy link
Member

krwq commented Mar 20, 2023

BOM is mainly used to check byte order - distinguish between little and big endian but UTF-8 doesn't have this problem. Also most Unix tools do not produce it and many don't expect it. Let's discuss it but this has already established pattern across dotnet/ repos...

@krwq krwq merged commit ec9df2a into dotnet:main Mar 23, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Dec 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-device-bindings Device Bindings for audio, sensor, motor, and display hardware that can used with System.Device.Gpio
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants