Skip to content

fix: support reopen with encoding in browser build.#79232

Closed
LeuisKen wants to merge 1 commit into
microsoft:masterfrom
LeuisKen:master
Closed

fix: support reopen with encoding in browser build.#79232
LeuisKen wants to merge 1 commit into
microsoft:masterfrom
LeuisKen:master

Conversation

@LeuisKen

@LeuisKen LeuisKen commented Aug 15, 2019

Copy link
Copy Markdown
Contributor

Hello, I've try the browser (or your call it web) build of vscode and it's awesome !

But I find some problems with "Reopen with Encoding", I've to work with some gbk encoded files but the browser build only supports utf8, so I write this patch to fix this problem and it can handle the readFile case.

However, to make it works on writeFile case, I need to import a third-party lib called text-encoding to handle the encoding of gbk strings since the standard TextEncoder API only support the utf8 encoding format. I don't know whether it's appropriate to import this library in vscode. So I just commit the readFile case patch in this pull request.

If you accept my solution for the writeFile case, I will commit the patch of it in this pull request.

Thanks for your work and waiting for your reply.

: )

Change-Id: I6b79a00edd96b0d9aaa7d7fef247f1a4db22a707
@bpasero

bpasero commented Aug 16, 2019

Copy link
Copy Markdown
Contributor

Thanks, but applying this patch results in an error on my end when I try it out.

Uncaught (in promise) RangeError: Failed to construct 'TextDecoder': The encoding label provided ('uft8') is invalid.

We will eventually support encodings in the web but currently I do not think I would accept a PR to resolve this. Thanks for the suggested change, but I will close this and revisit once we get there.

@bpasero

bpasero commented Aug 16, 2019

Copy link
Copy Markdown
Contributor

I opened #79275 for discussion.

@LeuisKen

LeuisKen commented Aug 16, 2019

Copy link
Copy Markdown
Contributor Author

Uncaught (in promise) RangeError: Failed to construct 'TextDecoder': The encoding label provided ('uft8') is invalid.

Well, it's my fault that didn't notice the incoming value should be utf-8 not utf8 (Although it works as javascript). And this problem should also be fixed by your team.

We will eventually support encodings in the web but currently I do not think I would accept a PR to resolve this. Thanks for the suggested change, but I will close this and revisit once we get there.

Actually I just want to report this bug as a pre-release user. 😂

@bpasero

bpasero commented Aug 16, 2019

Copy link
Copy Markdown
Contributor

Well, it's my fault that didn't notice the incoming value should be utf-8 not utf8 (Although it works as javascript). And this problem should also be fixed by your team.

What do you mean? When I change to utf-8 I still get this error:

Uncaught (in promise) RangeError: Failed to construct 'TextDecoder': The encoding label provided ('uft-8') is invalid.

@LeuisKen

LeuisKen commented Aug 16, 2019

Copy link
Copy Markdown
Contributor Author

WX20190816-145831

That's what I mean, it since goes wrong with typescript ? I'll test on my local environment now.

}

toString(): string {
toString(encoding: string = 'uft8'): string {

@LeuisKen LeuisKen Aug 16, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm so stupid to let utf8 be uft8.....
There is no excuse for this stupid mistake.......

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@LeuisKen Why don't you just fix this typo and force push to your branch...

@github-actions github-actions Bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants