Skip to content

Conversation

@lindexi
Copy link
Member

@lindexi lindexi commented Jun 3, 2020

No description provided.

@lindexi lindexi requested a review from a team as a code owner June 3, 2020 11:17
@ghost ghost added the PR metadata: Label to tag PRs, to facilitate with triage label Jun 3, 2020
@ghost ghost requested review from SamBent, fabiant3 and ryalanms June 3, 2020 11:17
@ryalanms ryalanms changed the title Fix unknow character Fix unknown character Jan 18, 2021
@ryalanms ryalanms merged commit bbd9e44 into dotnet:master Jan 18, 2021
@lindexi lindexi deleted the t/lindexi/fix-unknow-character branch January 19, 2021 01:00
@miloush
Copy link
Contributor

miloush commented Jan 25, 2021

I just want to point out that what needed to be fixed here is the encoding of the files.

The characters were valid quotation marks in 1252 codepage:
image

(so this might potentially break code that parses the error messages, although I agree that should not be encouraged)

@ryalanms ryalanms requested a review from a team January 25, 2021 19:34
@lindexi
Copy link
Member Author

lindexi commented Jan 26, 2021

@miloush Thank you. But I do not think we should use this character .

@ryalanms
Copy link
Member

We can revert this for now and reconsider for Preview 2. Any objections?

@lindexi
Copy link
Member Author

lindexi commented Jan 26, 2021

@ryalanms Thank you. Actually I found out that this problem was caused by garbled characters appearing on my device

@miloush
Copy link
Contributor

miloush commented Jan 26, 2021

I only wanted to point out that at least some of these were not some randomly garbled characters the way GitHub makes it look like. They don't seem to be applied consistently though (i.e. for specific languages) and also, after further checking, the XML files contain UTF-8 BOM which makes them mis-encoded, so it's just the txt file that had a reasonably working characters.

Furthermore, these are all ObjectReaderTypeCannotRoundtrip exception messages only which I find unlikely to be thrown for most application users, so if you are asking us to assess the risks, I would lean towards keeping this merged in and see if someone complains, especially if it's a preview.

@ghost ghost locked as resolved and limited conversation to collaborators Apr 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

PR metadata: Label to tag PRs, to facilitate with triage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants