fix: Provide a setting to define encoding for USS files. by amanPras · Pull Request #1237 · eclipse-che4z/che-che4z-lsp-for-cobol

amanPras · 2022-01-25T17:13:13Z

This should work for most of the cases but not all cases.

Signed-off-by: ap891843 aman.prashant@broadcom.com

amanPras · 2022-01-26T08:45:49Z

sample files for test:

bash-4.3$ pwd
/z/ap891843/CAPAX/Audit/TEST_COPYBOOKS/TESTCOPY
bash-4.3$
bash-4.3$ ls -T
t IBM-1047    T=on  EBCIDIC
t IBM-5352    T=on  IBM5352
t ISO8859-1   T=on  ISO8859
t ISO8859-1   T=on  ISO88591                 --> Has some Russian text for test
m IBM-1047    T=off UNTAGEBC
bash-4.3$

temanbrcom

🦾

grianbrcom

🥕

grianbrcom

Thanks, man.
🦩

VitGottwald · 2022-01-26T12:21:53Z

I am a little confused. I expected to have a setting that would specify the remote encoding:

If not specified, download would be in text mode.
If specified, download would be in binary and then a conversion from the specified encoding to UTF-8 would happen

Does that make sense?

amanPras · 2022-02-01T16:26:10Z

I am a little confused. I expected to have a setting that would specify the remote encoding:

If not specified, download would be in text mode.

If specified, download would be in binary and then a conversion from the specified encoding to UTF-8 would happen

Does that make sense?

Updated. Please review

amanPras · 2022-02-02T12:37:20Z

                    "description": "Default list of USS paths to search for copybooks\nZowe Explorer version 1.15.0 or higher is required to download copybooks from the mainframe",
                    "uniqueItems": true
                },
+                "cobol-lsp.cpy-manager.uss-copybook-encoding": {


Is it make sense to have a default value here?

I see, this is related to downloadBinary flag.

Yes, if nothing is provided, we download in text mode. In case encoding is provided we download in binary and make the conversion at our end.

grianbrcom

Can you, please, add a unit test with encoding? It will be better to add two: one with existing iconv-lite codepage and one with patched.

grianbrcom · 2022-02-03T13:40:08Z

                    "description": "Default list of USS paths to search for copybooks\nZowe Explorer version 1.15.0 or higher is required to download copybooks from the mainframe",
                    "uniqueItems": true
                },
+                "cobol-lsp.cpy-manager.uss-copybook-encoding": {


Is it make sense to have a default value here?

grianbrcom · 2022-02-03T13:49:05Z

@@ -0,0 +1,49 @@
+diff --git a/node_modules/iconv-lite/encodings/sbcs-data.js b/node_modules/iconv-lite/encodings/sbcs-data.js


Oh my... I hope that you know what are you doing =)

Can you, please, add a unit test with encoding? It will be better to add two: one with existing iconv-lite codepage and one with patched.

Updated PR

grianbrcom · 2022-02-03T13:50:17Z

+    "cp1047": "ebcdic1047",
+    "ebcdic1047": {
+        "type": "_sbcs",
+        "chars": "\u0000\u0001\u0002\u0003\u009C\u0009\u0086\u007F\u0097\u008D\u008E\u000B\u000B\u000C\u000E\u000F\u0010\u0011\u0012\u0013\u009D\u000A\u0008\u0087\u0018\u0019\u0092\u008F\u001C\u001D\u001E\u001F\u0080\u0081\u0082\u0083\u0084\u000A\u0017\u001B\u0088\u0089\u008A\u008B\u008C\u0005\u0006\u0007\u0090\u0091\u0016\u0093\u0094\u0095\u0096\u0004\u0098\u0099\u009A\u009B\u0014\u0015\u009E\u001A\u0020\u00A0\u00E2\u00E4\u00E0\u00E1\u00E3\u00E5\u00E7\u00F1\u00A2\u002E\u003C\u0028\u002B\u007C\u0026\u00E9\u00EA\u00EB\u00E8\u00ED\u00EE\u00EF\u00EC\u00DF\u0021\u0024\u002A\u0029\u003B\u005E\u002D\u002F\u00C2\u00C4\u00C0\u00C1\u00C3\u00C5\u00C7\u00D1\u00A6\u002C\u0025\u005F\u003E\u003F\u00F8\u00C9\u00CA\u00CB\u00C8\u00CD\u00CE\u00CF\u00CC\u0060\u003A\u0023\u0040\'\u003D\"\u00D8\u0061\u0062\u0063\u0064\u0065\u0066\u0067\u0068\u0069\u00AB\u00BB\u00F0\u00FD\u00FE\u00B1\u00B0\u006A\u006B\u006C\u006D\u006E\u006F\u0070\u0071\u0072\u00AA\u00BA\u00E6\u00B8\u00C6\u00A4\u00B5\u007E\u0073\u0074\u0075\u0076\u0077\u0078\u0079\u007A\u00A1\u00BF\u00D0\u005B\u00DE\u00AE\u00AC\u00A3\u00A5\u00B7\u00A9\u00A7\u00B6\u00BC\u00BD\u00BE\u00DD\u00A8\u00AF\u005D\u00B4\u00D7\u007B\u0041\u0042\u0043\u0044\u0045\u0046\u0047\u0048\u0049\u00AD\u00F4\u00F6\u00F2\u00F3\u00F5\u007D\u004A\u004B\u004C\u004D\u004E\u004F\u0050\u0051\u0052\u00B9\u00FB\u00FC\u00F9\u00FA\u00FF\u00E7\u00F7\u0053\u0054\u0055\u0056\u0057\u0058\u0059\u005A\u00B2\u00D4\u00D6\u00D2\u00D3\u00D5\u0030\u0031\u0032\u0033\u0034\u0035\u0036\u0037\u0038\u0039\u00B3\u00DB\u00DC\u00D9\u00DA\u009F"


That line is shorter than the other "chars". Was it done on purpose?

Thanks, I will double-check the conversion table for IBM-1047

Thanks, @grianbrcom. There was a mistake. Updated.

grianbrcom · 2022-02-03T13:52:13Z

                    "description": "Default list of USS paths to search for copybooks\nZowe Explorer version 1.15.0 or higher is required to download copybooks from the mainframe",
                    "uniqueItems": true
                },
+                "cobol-lsp.cpy-manager.uss-copybook-encoding": {


I see, this is related to downloadBinary flag.

amanPras · 2022-02-04T10:19:11Z

seems, to support zowe dependencies, we added ignore-script, which avoids post-install script to run ( patch-package relies on this). And that's why the build fails as the encoding test fails.

Trying to see what can be done :(

VitGottwald · 2022-02-04T16:45:29Z

@ap891843, does the encoding setting also apply to download from a dataset?

amanPras · 2022-02-07T12:35:03Z

@ap891843, does the encoding setting also apply to download from a dataset?

Updated to work with DSN's. Same settings is used for both DSN and USS for now.

temanbrcom · 2022-02-07T12:56:14Z

@@ -0,0 +1,49 @@
+diff --git a/node_modules/iconv-lite/encodings/sbcs-data.js b/node_modules/iconv-lite/encodings/sbcs-data.js


I still don't understand of this exact file

Well, okay. Anton explained it to me. Is it the only way to do that?

No actually.
We can raise a PR on iconv-lite. But I don't think it would be merged.
Vit showed me this PR from 2015 - pillarjs/iconv-lite#112

Signed-off-by: ap891843 <aman.prashant@broadcom.com>

temanbrcom

🧛

temanbrcom · 2022-02-07T14:03:45Z

+describe('Test iconv lib correctly decodes a binary buffer', () => {
+
+    it('checks iconv-lite decodes correctly for patched ebcdic encodings 1147 / 037/ 1047', () => {
+        expect(iconv.decode(Buffer.from([0xF3, 0xF0, 0xF0, 0x9F, 0x40, 0x40]), "cp1147")).toBe("300€  ");


Encoding not recognized: 'cp1147' (searched as: 'cp1147')

Yes. It's because the PR Jenkins file is not picked.
Anton tested this on https://ci.eclipse.org/che4z/job/LSP%20for%20COBOL/job/bugfix%252FUS798758-uss/

Oh sorry, forget to mention. Yes, it was tested properly in CI.

Let me run it for the latest changes.

grianbrcom

🥕

amanPras marked this pull request as ready for review January 26, 2022 08:42

amanPras commented Jan 26, 2022

View reviewed changes

Comment thread clients/cobol-lsp-vscode-extension/package.json Outdated

temanbrcom approved these changes Jan 26, 2022

View reviewed changes

temanbrcom requested a review from zeibura January 26, 2022 09:22

temanbrcom assigned amanPras Jan 26, 2022

temanbrcom added the bug Something isn't working label Jan 26, 2022

temanbrcom requested a review from grianbrcom January 26, 2022 09:23

amanPras force-pushed the bugfix/US798758-uss branch 2 times, most recently from 3d9a756 to 3214d63 Compare January 26, 2022 09:58

grianbrcom approved these changes Jan 26, 2022

View reviewed changes

Comment thread clients/cobol-lsp-vscode-extension/package.json Outdated

Comment thread clients/cobol-lsp-vscode-extension/src/services/Settings.ts Outdated

amanPras force-pushed the bugfix/US798758-uss branch from 3214d63 to 00189ed Compare January 26, 2022 10:13

amanPras requested a review from grianbrcom January 26, 2022 10:19

grianbrcom approved these changes Jan 26, 2022

View reviewed changes

amanPras marked this pull request as draft January 26, 2022 15:02

amanPras force-pushed the bugfix/US798758-uss branch 5 times, most recently from 3519e5e to 2852ae1 Compare February 1, 2022 16:22

amanPras marked this pull request as ready for review February 1, 2022 16:22

amanPras requested review from grianbrcom and temanbrcom February 1, 2022 16:22

amanPras force-pushed the bugfix/US798758-uss branch from 2852ae1 to ee7a817 Compare February 2, 2022 12:36

amanPras commented Feb 2, 2022

View reviewed changes

grianbrcom suggested changes Feb 3, 2022

View reviewed changes

amanPras force-pushed the bugfix/US798758-uss branch 2 times, most recently from 84b6a3f to 8fa5437 Compare February 4, 2022 09:41

amanPras requested a review from grianbrcom February 4, 2022 09:44

amanPras force-pushed the bugfix/US798758-uss branch from 8fa5437 to 9dcf75e Compare February 4, 2022 10:53

amanPras closed this Feb 4, 2022

amanPras reopened this Feb 4, 2022

amanPras force-pushed the bugfix/US798758-uss branch from 9dcf75e to 5b1861f Compare February 7, 2022 12:33

temanbrcom reviewed Feb 7, 2022

View reviewed changes

amanPras force-pushed the bugfix/US798758-uss branch 2 times, most recently from eb6ecc4 to fe48ef0 Compare February 7, 2022 13:23

fix: Provide a setting to define encoding for USS files.

62edeaf

Signed-off-by: ap891843 <aman.prashant@broadcom.com>

amanPras force-pushed the bugfix/US798758-uss branch from fe48ef0 to 62edeaf Compare February 7, 2022 13:27

temanbrcom approved these changes Feb 7, 2022

View reviewed changes

temanbrcom reviewed Feb 7, 2022

View reviewed changes

grianbrcom approved these changes Feb 8, 2022

View reviewed changes

temanbrcom merged commit cb6a4e4 into eclipse-che4z:development Feb 8, 2022

amanPras mentioned this pull request Feb 8, 2022

fix: Fetch copybooks from DSN first. #1253

Closed

amanPras deleted the bugfix/US798758-uss branch May 27, 2024 14:35

		@@ -0,0 +1,49 @@
		diff --git a/node_modules/iconv-lite/encodings/sbcs-data.js b/node_modules/iconv-lite/encodings/sbcs-data.js

Uh oh!

Conversation

amanPras commented Jan 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

amanPras commented Jan 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

temanbrcom left a comment

Choose a reason for hiding this comment

Uh oh!

grianbrcom left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

grianbrcom left a comment

Choose a reason for hiding this comment

Uh oh!

VitGottwald commented Jan 26, 2022

Uh oh!

amanPras commented Feb 1, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

grianbrcom left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amanPras commented Feb 4, 2022

Uh oh!

VitGottwald commented Feb 4, 2022

Uh oh!

amanPras commented Feb 7, 2022

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

temanbrcom left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

grianbrcom left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

amanPras commented Jan 25, 2022 •

edited

Loading

amanPras commented Jan 26, 2022 •

edited

Loading