Skip to content

Convert input handling based on CCSID scenarios #2556

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

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

worksofliam
Copy link
Contributor

Enhance input conversion logic to account for CCSID in various contexts, ensuring proper translation for system commands and SQL statements. Adjustments made to improve path handling and user feedback during checks.

Fixes #2519

How to test

  1. List members/objects in the Object Browser with variant chats
  2. Open source members up
  3. Compile source members
  4. Run the test cases

…arios.

Signed-off-by: worksofliam <mrliamallan@live.co.uk>
Copy link
Contributor

github-actions bot commented Mar 6, 2025

👋 A new build is available for this PR based on 79e49f1.

Signed-off-by: worksofliam <mrliamallan@live.co.uk>
@worksofliam worksofliam temporarily deployed to testing_environment March 6, 2025 16:45 — with GitHub Actions Inactive
@worksofliam worksofliam temporarily deployed to testing_environment March 6, 2025 17:38 — with GitHub Actions Inactive
Signed-off-by: worksofliam <mrliamallan@live.co.uk>
Signed-off-by: worksofliam <mrliamallan@live.co.uk>
Signed-off-by: worksofliam <mrliamallan@live.co.uk>
@worksofliam worksofliam temporarily deployed to testing_environment March 6, 2025 19:59 — with GitHub Actions Inactive
Signed-off-by: worksofliam <mrliamallan@live.co.uk>
@worksofliam worksofliam temporarily deployed to testing_environment March 7, 2025 14:55 — with GitHub Actions Inactive
Signed-off-by: worksofliam <mrliamallan@live.co.uk>
@worksofliam worksofliam temporarily deployed to testing_environment March 7, 2025 15:14 — with GitHub Actions Inactive
@worksofliam worksofliam marked this pull request as draft March 28, 2025 14:34
Tools.qualifyPath(inAmerican(file.library), inAmerican(file.name), inAmerican(member), asp, true),
Tools.qualifyPath(inAmerican(file.library), inAmerican(file.name), inAmerican(member), undefined, true)
Tools.qualifyPath(forSystem(file.library), forSystem(file.name), forSystem(member), asp, localVariants),
Tools.qualifyPath(fromSystem(file.library), fromSystem(file.name), fromSystem(member), undefined, localVariants)
Copy link
Collaborator

Choose a reason for hiding this comment

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

It used to be inAmerican, so I think this should be forSystem here.

Suggested change
Tools.qualifyPath(fromSystem(file.library), fromSystem(file.name), fromSystem(member), undefined, localVariants)
Tools.qualifyPath(fromSystem(file.library), forSystem(file.name), forSystem(member), undefined, localVariants)

Comment on lines +1262 to 1277
sysNameInAmerican(string: string, enabled: boolean = true) {
if (enabled) {
const fromChars = this.variantChars.local;
const toChars = this.variantChars.american;

let result = string;
let result = string;

for (let i = 0; i < fromChars.length; i++) {
result = result.replace(new RegExp(`[${fromChars[i]}]`, `g`), toChars[i]);
};
for (let i = 0; i < fromChars.length; i++) {
result = result.replace(new RegExp(`[${fromChars[i]}]`, `g`), toChars[i]);
};

return result;
}

return result
return string;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

The logic behind the enabled parameter is unclear or would need to be redefined. Looking at all the references in the code, it's always this class' qsysPosixPathsRequireTranslation field that is used.

Here is a suggestion: make that second parameter optional, and when it's true, consider qsysPosixPathsRequireTranslation value. Then in the places where the method is called with the second parameter, pass ifNeeded as true:

Suggested change
sysNameInAmerican(string: string, enabled: boolean = true) {
if (enabled) {
const fromChars = this.variantChars.local;
const toChars = this.variantChars.american;
let result = string;
let result = string;
for (let i = 0; i < fromChars.length; i++) {
result = result.replace(new RegExp(`[${fromChars[i]}]`, `g`), toChars[i]);
};
for (let i = 0; i < fromChars.length; i++) {
result = result.replace(new RegExp(`[${fromChars[i]}]`, `g`), toChars[i]);
};
return result;
}
return result
return string;
}
sysNameInAmerican(string: string, ifNeeded?: boolean) {
if (!ifNeeded || this.qsysPosixPathsRequireTranslation) {
const fromChars = this.variantChars.local;
const toChars = this.variantChars.american;
let result = string;
for (let i = 0; i < fromChars.length; i++) {
result = result.replace(new RegExp(`[${fromChars[i]}]`, `g`), toChars[i]);
};
return result;
}
return string;
}

Copy link
Collaborator

@sebjulliand sebjulliand left a comment

Choose a reason for hiding this comment

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

@worksofliam I made an initial code review. I'll see how it goes at runtime.
I left a few comments for some changes. Thanks!

@sebjulliand
Copy link
Collaborator

sebjulliand commented Apr 25, 2025

Tested with CCSID 297: listing files and members from a single library with variants work.
image

Opening a member whose path contains variants doesn't work. When source dates are enabled, the SQL alias is created with American variants:

CREATE OR REPLACE ALIAS ILEDITOR.TEMP_E3D5CD600772FF279A8AF823A8DBB3901555096A for "@MAZING"."H@RRIBLE"("H@L@#@$")

It used to be created with the locale's variants:

CREATE OR REPLACE ALIAS ILEDITOR.TEMP_6790C94968740AB09E5F395BAFD5AC15037BB33A for "àMAZING"."HàRRIBLE"("HàLP£à$")

Listing libraries with a variant doesn't work:
image

it used to list four libraries:
image

The reason is the SQL query used to use the variant chars:

select    OBJNAME          as NAME,   OBJTYPE          as TYPE,   OBJATTRIBUTE     as ATTRIBUTE,   OBJTEXT          as TEXT,   0                as IS_SOURCE,   IASP_NUMBER      as IASP_NUMBER,   OBJSIZE          as SIZE,   extract(epoch from (OBJCREATED))*1000       as CREATED,   extract(epoch from (CHANGE_TIMESTAMP))*1000 as CHANGED,   OBJOWNER         as OWNER,   OBJDEFINER       as CREATED_BY from table(QSYS2.OBJECT_STATISTICS(OBJECT_SCHEMA => 'QSYS', OBJTYPELIST => '*LIB', OBJECT_NAME => 'à*'))

With the PR, it doesn't:

select    OBJNAME          as NAME,   OBJTYPE          as TYPE,   OBJATTRIBUTE     as ATTRIBUTE,   OBJTEXT          as TEXT,   0                as IS_SOURCE,   IASP_NUMBER      as IASP_NUMBER,   OBJSIZE          as SIZE,   extract(epoch from (OBJCREATED))*1000       as CREATED,   extract(epoch from (CHANGE_TIMESTAMP))*1000 as CHANGED,   OBJOWNER         as OWNER,   OBJDEFINER       as CREATED_BY from table(QSYS2.OBJECT_STATISTICS(OBJECT_SCHEMA => 'QSYS', OBJTYPELIST => '*LIB', OBJECT_NAME => '@*')

Three encoding tests fail with this config:
image

image

 FAIL  tests/suites/encoding.test.ts > Encoding tests > Listing objects with variants
AssertionError: expected undefined to be truthy

- Expected: 
true

+ Received: 
undefined

 ❯ tests/suites/encoding.test.ts:223:33
    221|       if (!skipLibrary) {
    222|         const [expectedLibrary] = await content.getLibraries({ library });
    223|         expect(expectedLibrary).toBeTruthy();
       |                                 ^
    224|         expect(library).toBe(expectedLibrary.name);
    225| 

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[7/9]⎯

 FAIL  tests/suites/encoding.test.ts > Encoding tests > Library list supports dollar sign variant
Error: Failed uploading member: Error: Erreurs trouvées dans la commande. (38501)
 ❯ IBMiContent.uploadMemberContent IBMiContent.ts:291:15
    289|         return true;
    290|       } else {
    291|         throw new Error(`Failed uploading member: ${copyResult.stderr}`);
       |               ^
    292|       }
    293|     } catch (error) {
 ❯ tests/suites/encoding.test.ts:290:7

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[8/9]⎯

 FAIL  tests/suites/encoding.test.ts > Encoding tests > Variant character in source names and commands
Error: Failed uploading member: Error: Erreurs trouvées dans la commande. (38501)
 ❯ IBMiContent.uploadMemberContent IBMiContent.ts:291:15
    289|         return true;
    290|       } else {
    291|         throw new Error(`Failed uploading member: ${copyResult.stderr}`);
       |               ^
    292|       }
    293|     } catch (error) {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

On one IBMi I can't open a source member with '£' in the member's name, but on another one it works
2 participants