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.

Sign up for GitHub

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

Jump to bottom

Encoding euckr does not exist anymore #101847

Closed
bpasero opened this issue Jul 7, 2020 · 5 comments
Closed

Encoding euckr does not exist anymore #101847

bpasero opened this issue Jul 7, 2020 · 5 comments
Assignees
@bpasero
Labels
bug Issue identified by VS Code Team member as probable bug candidate Issue identified as probable candidate for fixing in the next release file-encoding File encoding type issues regression Something that used to work is now broken verified Verification succeeded
Milestone
June 2020

Comments

@bpasero
Copy link
Member

bpasero commented Jul 7, 2020

@gyzerok this seems to be a regression in the webpack version of iconv-lite, I am pushing a test now to review...
@bpasero bpasero added file-encoding File encoding type issues regression Something that used to work is now broken labels Jul 7, 2020
@bpasero bpasero added this to the July 2020 milestone Jul 7, 2020
@bpasero bpasero self-assigned this Jul 7, 2020
bpasero added a commit that referenced this issue Jul 7, 2020
@bpasero
encodings - failing test for #101847
c212efa
@bpasero
Copy link
Member Author

bpasero commented Jul 7, 2020

Uncomment this line to see the failure:

vscode/src/vs/workbench/services/textfile/test/node/encoding/encoding.test.ts

Lines 410 to 412 in c212efa

if (enc === 'euckr') {
continue; // TODO@ben failing test for https://github.com/microsoft/vscode/issues/101847
}

When I debug this I see the following exception for encodingExists:

TypeError: Cannot read property '63' of undefined

a (/Users/bpasero/Development/Microsoft/monaco/node_modules/iconv-lite-umd/lib/iconv-lite-umd.js:10)
a.getCodec (/Users/bpasero/Development/Microsoft/monaco/node_modules/iconv-lite-umd/lib/iconv-lite-umd.js:8)
a.encodingExists (/Users/bpasero/Development/Microsoft/monaco/node_modules/iconv-lite-umd/lib/iconv-lite-umd.js:8)
encodingExists (encoding.js:136)
await in encodingExists (async)
getEncodingForResource (textFileService.js:478)
getReadEncoding (textFileService.js:464)
overwriteEncoding (textFileService.js:80)
createDecoder (encoding.js:40)
await in createDecoder (async)
(anonymous) (encoding.js:74)
(anonymous) (stream.js:170)
flowData (stream.js:170)
resume (stream.js:56)
on (stream.js:122)
(anonymous) (encoding.js:61)
toDecodeStream (encoding.js:27)
doRead (textFileService.js:78)
await in doRead (async)
read (textFileService.js:54)
read (nativeTextFileService.js:28)
run (editorStatus.js:1116)
await in run (async)
triggerAndDisposeAction (actions.js:85)
await in triggerAndDisposeAction (async)
(anonymous) (actions.js:64)
invokeFunction (instantiationService.js:43)
_tryExecuteCommand (commandService.js:74)
(anonymous) (commandService.js:64)
Promise.then (async)
executeCommand (commandService.js:64)
executeCommand (statusbarPart.js:639)
(anonymous) (statusbarPart.js:594)

@gyzerok @ashtuchkin any clues?

@bpasero bpasero added the bug Issue identified by VS Code Team member as probable bug label Jul 7, 2020
@gyzerok
Copy link
Contributor

gyzerok commented Jul 7, 2020 •
edited

@bpasero I can take a look tomorrow. Quick idea - this is because of cp949 exclusion.

I was trying to reduce the library size by removing what is not needed. It seems that euckr is part of cp949 and I missed it.

@bpasero
Copy link
Member Author

bpasero commented Jul 7, 2020

@gyzerok ok thanks. one thought I had: it would maybe be good if iconv-lite-umd would run the test suite of iconv-lite somehow to validate things are working fine. Maybe reduced depending on what VSCode is using.

@bpasero bpasero added the candidate Issue identified as probable candidate for fixing in the next release label Jul 7, 2020
@bpasero bpasero modified the milestones: July 2020, June 2020 Jul 7, 2020
@bpasero bpasero reopened this Jul 7, 2020
pull bot pushed a commit to nonomal/vscode that referenced this issue Jul 7, 2020
@bpasero
Encoding euckr does not exist anymore (fix microsoft#101847)
d95b654
bpasero added a commit that referenced this issue Jul 7, 2020
@bpasero
Encoding euckr does not exist anymore (fix #101847)
7d28806
@bpasero bpasero mentioned this issue Jul 7, 2020
Merged
@bpasero
Copy link
Member Author

bpasero commented Jul 7, 2020

@gyzerok I ended up simply doing this change which makes the test succeed:

microsoft/vscode-iconv-lite-umd@f4180d8

@bpasero bpasero closed this as completed Jul 7, 2020
bpasero added a commit that referenced this issue Jul 7, 2020
@bpasero
Encoding euckr does not exist anymore (fix #101847) (#101853)
68079a9
gjsjohnmurray pushed a commit to gjsjohnmurray/vscode that referenced this issue Jul 8, 2020
@bpasero @gjsjohnmurray
encodings - failing test for microsoft#101847
21fb814
gjsjohnmurray pushed a commit to gjsjohnmurray/vscode that referenced this issue Jul 8, 2020
@bpasero @gjsjohnmurray
Encoding euckr does not exist anymore (fix microsoft#101847)
430b34a
gjsjohnmurray pushed a commit to gjsjohnmurray/vscode that referenced this issue Jul 9, 2020
@bpasero @gjsjohnmurray
encodings - failing test for microsoft#101847
8ef2708
gjsjohnmurray pushed a commit to gjsjohnmurray/vscode that referenced this issue Jul 9, 2020
@bpasero @gjsjohnmurray
Encoding euckr does not exist anymore (fix microsoft#101847)
36951bb
@dbaeumer
Copy link
Member

dbaeumer commented Jul 9, 2020

Verified using the following steps:

  • open this file: build/win32/i18n/Default.ko.isl
  • in the status bar switch to encoding EUC-KR

Verify that encoding is picked

@dbaeumer dbaeumer added the verified Verification succeeded label Jul 9, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Aug 21, 2020
lemanschik pushed a commit to code-oss-dev/code that referenced this issue Nov 25, 2022
@bpasero
Encoding euckr does not exist anymore (fix microsoft#101847)
dddb462
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@bpasero bpasero

Labels
bug Issue identified by VS Code Team member as probable bug candidate Issue identified as probable candidate for fixing in the next release file-encoding File encoding type issues regression Something that used to work is now broken verified Verification succeeded
Projects
None yet
Milestone
June 2020
Development

No branches or pull requests
3 participants
@bpasero @gyzerok @dbaeumer