Skip to content

Fix ThirdPartyNoticeText.txt encoding #53184

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

Conversation

jakebailey
Copy link
Member

This is #52800 but using the method I outlined in #52800 (comment).

I didn't send this before as I figured the original author might want to do it, but that didn't happen and the PR timed out.

Takes a minute to send, so, here it is.

@jakebailey jakebailey requested a review from RyanCavanaugh March 9, 2023 19:22
@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Mar 9, 2023
@@ -162,7 +162,7 @@ For purposes of this definition, the normative portions of the Specification sha
10.7.8. shall not be considered accepted by an implementer who manifests an intent not to accept the terms of the W3C Community RF Licensing Requirements license as offered by the licensor.
10.7.9. The RF license conforming to the requirements in this policy shall be made available by the licensor as long as the Specification is in effect. The term of such license shall be for the life of the patents in question.
I am encouraged to provide a contact from which licensing information can be obtained and other relevant licensing information. Any such information will be made publicly available.
10.8. You or Your. You,� �you, or your means any person or entity who exercises copyright or patent rights granted under this Agreement, and any person that person or entity controls.
10.8. You or Your. You,” “you, or your means any person or entity who exercises copyright or patent rights granted under this Agreement, and any person that person or entity controls.

Choose a reason for hiding this comment

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

I'm not sure this is an improvement... 😉

Choose a reason for hiding this comment

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

Okay, I think GitHub's diff display is just bugged. It looks fine in my review comment, but under the Files Changed tab all the non-ASCII characters are mangled, whereas the original looks normal. Does this file need a BOM or something?

Copy link
Member Author

Choose a reason for hiding this comment

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

If that's what the license includes, that's what we include. I'm definitely not going to start arbitrarily replacing characters in someone else's license!

Choose a reason for hiding this comment

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

See my second comment. I wasn't implying you should edit out the smartquotes.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this file was so broken before that nothing could tell what it was, hence fixing the encoding to UTF-8 without BOM.

Copy link
Member Author

Choose a reason for hiding this comment

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

See my second comment. I wasn't implying you should edit out the smartquotes.

Hah, yeah, I didn't realize that the other diff view got this wrong.

@@ -85,12 +85,12 @@ DOM
W3C License
This work is being provided by the copyright holders under the following license.
By obtaining and/or copying this work, you (the licensee) agree that you have read, understood, and will comply with the following terms and conditions.
Permission to copy, modify, and distribute this work, with or without modification,for any purpose and without fee or royalty is hereby granted, provided that you include the following
Permission to copy, modify, and distribute this work, with or without modification, for any purpose and without fee or royalty is hereby granted, provided that you include the following

Choose a reason for hiding this comment

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

image

No idea what's going on here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Meanwhile in Outlook 😃

image

Choose a reason for hiding this comment

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

I'm starting to think this file is actually cursed.

Copy link
Member

Choose a reason for hiding this comment

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

There's actually an 0xC2 byte at this position
image

Copy link
Member Author

Choose a reason for hiding this comment

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

And here I thought I had cracked the code.

@jakebailey
Copy link
Member Author

Well, it's probably right. I'm going to merge and then we can look at the file afterwards instead, because github sure isn't doing well here.

@jakebailey jakebailey merged commit 39d9fdc into microsoft:main Mar 9, 2023
@jakebailey jakebailey deleted the fix-third-party-notice-text-encoding branch March 9, 2023 21:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants