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.

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

Sources with "à" #1935

Closed
GCast31 opened this issue Mar 14, 2024 · 17 comments · Fixed by #1940 or #1941
Closed

Sources with "à" #1935

GCast31 opened this issue Mar 14, 2024 · 17 comments · Fixed by #1940 or #1941
Labels
bug A confirmed issue when something isn't working as intended

Comments

@GCast31
Copy link

GCast31 commented Mar 14, 2024

Hi,

We have our sources named with "à" in last position (ex. CUSTOMà) but we can't open this file because it seem to do an uppercase

How can we do for using extension for our sources ?

Thanks

@sebjulliand
Copy link
Collaborator

Hi @GCast31 ,
I reproduced this on my end and I can confirm this is a bug.
I'll try to get it fixed soon.

@worksofliam
Copy link
Contributor

@sebjulliand Can you show me how to recreate this sometime using CL? I think this is the kind of thing we should write a test case for.

@sebjulliand
Copy link
Collaborator

@sebjulliand Can you show me how to recreate this sometime using CL? I think this is the kind of thing we should write a test case for.

Sure thing @worksofliam .

BTW, I almost fixed it by removing all the toUpperCase applied to library, file and member names. But "almost" because then CPYTOSTMF will not allow that à in the name. But I have a possible solution to this.

@worksofliam worksofliam added the bug A confirmed issue when something isn't working as intended label Mar 14, 2024
@chrjorgensen
Copy link
Collaborator

@sebjulliand I would like to know more about this bug:

  • what is the CCSID?
  • what is the connection setting for Use SQL?

We must be careful when changing code handling object / member names with variant characters... we risk breaking things because of the many different situations. I've never seen a report of names with à before, so this situation is new - maybe an unusual CCSID?

@sebjulliand
Copy link
Collaborator

@chrjorgensen sure.

Since à is a common character found in French, I went with CCSID 297 and PASE_LANG FR_FR. SQL was disabled on this connection.

It happens that à is the variant character for @. (c.f. variantChars field of IBMi.ts once connected).

It is valid when used as an object name. And so uppercasing it causes this kind of issue. I already saw some customers using å with Danish or Norwegian CCSID. It would cause the same issue.

We must be careful when changing code handling object / member names with variant characters... we risk breaking things because of the many different situations.

That is true, but we will need to support this use case I think. It's unusual but legit, I guess.

@chrjorgensen
Copy link
Collaborator

@sebjulliand

It happens that à is the variant character for @. (c.f. variantChars field of IBMi.ts once connected).

Exactly what I was looking for. 👍

That is true, but we will need to support this use case I think. It's unusual but legit, I guess.

Sure, no complaint from here. 😉 All local characters caused by using variant characters in object names must be supported.

It is valid when used as an object name. And so uppercasing it causes this kind of issue. I already saw some customers using å with Danish or Norwegian CCSID. It would cause the same issue.

No - the variant characters in CCSID 277 (Danish/Norwegian) are the uppercase characters ÆØÅ. Removing the uppercase function would break things for CCSID 277!

The real issue is that CCSID 297 has a lowercase variant character à, which should not be uppercased with the remaining characters in the object name.

(You French people... why did you choose a lowercase character for @?! Oh, the Americans chose it for you? Right, sorry! 😆)

Seems like we can't rely on simple upper-lowercasing of object names, but have to implement a conversion based on CCSID ourselves... Right?

@sebjulliand
Copy link
Collaborator

(You French people... why did you choose a lowercase character for @?! Oh, the Americans chose it for you? Right, sorry! 😆)

You've got me laughing out loud for a solid minute here 🤣

Seems like we can't rely on simple upper-lowercasing of object names, but have to implement a conversion based on CCSID ourselves... Right?

You're on point! How about we make a function that uppercases only A-Z characters and eligible variant characters? (i.e. testà would become TESTà and testå would become TESTÅ). What do you think?

@chrjorgensen
Copy link
Collaborator

How about we make a function that uppercases only A-Z characters and eligible variant characters? (i.e. testà would become TESTà and testå would become TESTÅ).

I will check all CCSID's but I think only a few of them have letters as variant characters. It should be fairly easy to create a table of the eligible variant characters for the conversion function.

@chrjorgensen
Copy link
Collaborator

I'm sorry to report that France was the only country hit by the Americans by having a lowercase variant character... 😞
For the result of my analysis, see this Google Sheet.

But that means we only have to make special consideration for the à character..! 🎉

@sebjulliand Will you make the routine to convert only A-Z and the eligible variant characters, as you suggested? 🙏

@sebjulliand
Copy link
Collaborator

I'm sorry to report that France was the only country hit by the Americans by having a lowercase variant character... 😞 For the result of my analysis, see this Google Sheet.

But that means we only have to make special consideration for the à character..! 🎉

@sebjulliand Will you make the routine to convert only A-Z and the eligible variant characters, as you suggested? 🙏

Oh dear 🙄 Let's kick the French out of the platform and...no...wait...
I'll create that function, no problem 😁

@sebjulliand
Copy link
Collaborator

@chrjorgensen I opened PR #1940 with the new function to upperCase names.
However, it's still a draft because one problem remains: CPYTOSTMF doesn't like this variant character at all.

For example, with PR applied we can get to the point where CPYTOSTMF gets executed:

CPYTOSTMF FROMMBR('/QSYS.LIB/XXSJULLIAN.LIB/QCLSRC.FILE/COUCOUà.MBR') TOSTMF('/tmp/vscodetemp-O_RMRWIcJX') STMFOPT(*REPLACE) STMFCCSID(1208) DBFCCSID(*FILE)

And it returns this error (translated from French):

CPF4102: COUCOUÀ member of XXSJULLIAN QCLSRC file not found.
CPDA084: Unable to open file.
CPF4404: File QCLSRC was already closed, never opened or not by this process.
CPFA097: Object not copied. The object is /QSYS.LIB/XXSJULLIAN.LIB/QCLSRC.FILE/COUCOUà.MBR.

I check from WRKLNK how these paths with variants were handled and they get replaced by their american counterparts (sorry, French screen...but you get the idea):
image

In this case, the CPY command will accept this path:

CPY OBJ('/QSYS.LIB/XXSJULLIAN.LIB/QCLSRC.FILE/[email protected]')                             

This works...but not with CPYTOSTMF 😭 Even replacing à with @ doesn't help...so the issue is partially fixed by the PR.

At this point, in my opinion, we have two possible solutions:

  • We wait for your new SQL procedure to read members. I assume SQL handles the variant correctly. But we will probably need an SQL procedure to write members as well as I suspect CPYFRMSTMF will crash the same.
  • We make a temporary copy of this member, with a correct name, and open it. And we do the same when saving the member.

What do you think @chrjorgensen, @worksofliam ? 🤔

@worksofliam
Copy link
Contributor

worksofliam commented Mar 18, 2024

@sebjulliand I hate to say it, but it seems like the first option is the way forward: copy to a temporary member, then open it. The reason is: the current SQL method is a lot slower than CPYTOSTMF and for that reason I use it mostly.

I wonder why CPYTOSTMF will not accept the variant?

@sebjulliand
Copy link
Collaborator

@worksofliam agreed. It's already in the PR 😅

@sebjulliand
Copy link
Collaborator

Hi @GCast31 ,
your issue has been fixed and the version 2.8.3 pre-release should be available in a few minutes.
If you switch to pre-release version of the extension in VSCode, you'll be able to open your source member with à right away. Otherwise you'll have to wait for the next release 😊

@GCast31
Copy link
Author

GCast31 commented Mar 19, 2024

Hi @sebjulliand

Thanks for your fix but we have some problems. We can now open sources files but not compile. When we try to create a new member TESTà.RPGLE for example (Error : Invalid Member Name : TESTÀ)

@sebjulliand
Copy link
Collaborator

@GCast31 can you share the error you have when compiling (from the output tab)? What action did you use?
I tried with the standard action and it compiled fine:

Running Action: Create Bound CL Program (CRTBNDCL) (10:39:43 AM)
Current library: XXSJULLIAN
Library list: XXSJUASP QGPL QTEMP
Working directory: /home/SJULLIAND
Commands:
                CRTBNDCL PGM(XXSJULLIAN/TESTà) SRCFILE(XXSJULLIAN/QCLSRC) OPTION(*EVENTF) DBGVIEW(*SOURCE)
...

Meanwhile, I'll fix the creation issue.

@chrjorgensen
Copy link
Collaborator

@GCast31 A new pre-release version 2.8.4 has just been created, which includes the latest changes for the à problem - please give it a spin... 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A confirmed issue when something isn't working as intended
Projects
None yet
4 participants