Member resolves, IFS resolves and local `includePath` support #225

worksofliam · 2023-06-15T09:48:47Z

Changes

Include and copy statements that have a member path will now resolve via the library list.
Include and copy statements that have a unix style path from a local file will now resolve via the includePath in iproj.json.
Streamfiles and local sources can now support member include paths correctly (valid in RPG)

Checklist

have tested my change
updated relevant documentation
Remove any/all console.logs I added
eslint is not complaining
have added myself to the contributors' list in the README
for feature PRs: PR only includes one feature enhancement.

Signed-off-by: Liam Allan <mrliamallan@live.co.uk>

chrjorgensen · 2023-06-18T20:47:44Z

@worksofliam I was curious and went through your code for include and copy of members. And I noticed some things, that is not how the RPG compiler works (according to the documentation):

If no source file is specified, you assume QRPGLEREF - but the compiler searches QRPGLESRC, not QRPGLEREF!
If a member is found, you add the extension .rpgleinc - but the member extension is not known if shell search is performed, only if SQL is used at a later point in time.
Extension .rpgleinc is only used when a member is not found and IFS files are searched for. Then IFS files with the name and extension specified is searched for, or if no extension is specified, the name with extensions .rpgle and .rpgleinc.

It seems like there's some confusion about this subject, probably because of different conventions and company standards. But IMHO we should follow the rules laid out by the RPG compiler. ➡️

worksofliam · 2023-06-18T21:16:10Z

@chrjorgensen wow, great write up!

Whoops! Simple mistake on my part. I can correct to QRPGLESRC!
We use rpgleinc just to give the member an extension. At resolve time, the extension isn't import, but we use it just to signify it is an include.
I will fix up those IFS resolve inconsistencies.

Changes incoming over the next day or two with your suggestions. Thanks you!!

Signed-off-by: Liam Allan <mrliamallan@live.co.uk>

worksofliam · 2023-06-23T20:33:58Z

@chrjorgensen Based on your feedback, the only change I made is to use QRPGLESRC instead of QRPGLEREF.

The plan is to first get member resolves working, and then move onto IFS/local.

Signed-off-by: Liam Allan <mrliamallan@live.co.uk>

worksofliam · 2023-06-23T21:46:22Z

@chrjorgensen Further updates to improve the code and allow cross file system resolves and cleaner code. I updated the PR description to reflect all new changes.

chrjorgensen

@worksofliam

I tested by creating a library TESTPR225 and using the following RPGLE main source with /copy and /include statements from the documentation:

**free

/COPY MBR1
/INCLUDE SRCFIL,MBR2
/COPY TESTPR225/SRCFIL,MBR3
/INCLUDE TESTPR225/"SRC>3","MBR¬3"
/COPY /dir1/dir2/file.rpg
/COPY /dir1/dir2/file
/COPY dir1/dir2/file.rpg¬3"
/COPY "ifs file containing blanks"
/COPY 'ifs file containing blanks'

I tested the member / streamfile resolve by putting the cursor on the name and press Alt-F12 to peek the content of the include file.

I noted the following:

The copy of MBR1 worked fine, member found was TESTPR225/QRPGLESRC/MBR1.RPGLE.
The include of SRCFIL/MBR2 worked fine, member found was TESTPR225/SRCFIL/MBR2.RPGLE.
The copy of TESTPR225/SRCFIL/MBR3 worked fine, member found was TESTPR225/SRCFIL/MBR3.RPGLE.
The include of TESTPR225/"SRC>3","MBR¬3" could not be resolved. Compile found it.
The IFS copy of /dir1/dir2/file.rpg could not be resolved. Compile found it.
The IFS copy of /dir1/dir2/file could not be resolved. Compile found it.
The IFS copy of dir1/dir2/file.rpg could not be resolved. Compile found it.
The IFS copy of "ifs file containing blanks" could not be resolved. Compile found it.
The IFS copy of 'ifs file containing blanks' could not be resolved. Compile found it.
The member resolve worked both when TESTPR225 was the current library or in the library list (I had it as the last library in the list with a prior library also containing a source file called SRCFIL).

Changing the current library also didn't affect the resolve cache - I had to disconnect and reconnect to make the resolve work, after first having another current library than TESTPR225.

Should there be some command or other way of resetting the cache? Would be nice for the user to be able to do that, when the cache has old or obsolete content... or at least it should be cleared when the current library or library list changes...

Signed-off-by: Liam Allan <mrliamallan@live.co.uk>

github-actions · 2023-06-29T20:48:35Z

👋 A new build is available for this PR based on 6178f1d.

Signed-off-by: Liam Allan <mrliamallan@live.co.uk>

github-actions · 2023-06-29T21:27:05Z

👋 A new build is available for this PR based on 173bfb7.

worksofliam · 2023-06-29T21:30:55Z

@chrjorgensen

The include of TESTPR225/"SRC>3","MBR¬3" could not be resolved. Compile found it.
- No plans to implement this style.
The IFS copy of /dir1/dir2/file.rpg could not be resolved. Compile found it.
- This has been done. Explicit path started from root (/) will now resolve.
The IFS copy of /dir1/dir2/file could not be resolved. Compile found it.
- Now works when path is relative.
The IFS copy of dir1/dir2/file.rpg could not be resolved. Compile found it.
- This has been done. If not root (/) is provided, it will search relative of the working directory (which can be changed in the IFS path)
The IFS copy of "ifs file containing blanks" could not be resolved. Compile found it.
- Support for double quotes is in.
The IFS copy of 'ifs file containing blanks' could not be resolved. Compile found it.
- I think this should work now.

Changing the current library also didn't affect the resolve cache - I had to disconnect and reconnect to make the resolve work, after first having another current library than TESTPR225.

You can simple close and re-open the member/IFS/local file now.

I will post again when the remaining items are complete.

worksofliam · 2023-06-29T23:17:14Z

Great progress has been made.

@chrjorgensen Back to you for another review whenever you can.

Signed-off-by: Liam Allan <mrliamallan@live.co.uk>

github-actions · 2023-06-30T00:07:31Z

👋 A new build is available for this PR based on 7dfe7f7.

chrjorgensen

I did some further testing on the same test source:

❌ The copy of TESTPR225/SRCFIL,MBR3 no longer worked - your code probably thinks it's a UNIX path, when it's actually a QSYS path.
✔️ The IFS copy of /dir1/dir2/file.rpg worked now.
❌ The IFS copy of /dir1/dir2/file worked - partially. Could only be resolved, when exact filename existed, but no seatch for suffixes .rpgle and .rpgleinc is performed.
✔️ The IFS copy of dir1/dir2/file.rpg worked now.
❌ I could not get the IFS copy of "ifs file containing blanks" to work.
❌ I could not get the IFS copy of 'ifs file containing blanks' to work.

You should probably also take into consideration the rules for when the compiler can't determine the filesystem specified, as described here: https://www.ibm.com/docs/en/i/7.3?topic=files-include. Then both QSYS and IFS will be searched as documented. I see that you already commented on this in the code, but it seems not implemented yet?

chrjorgensen · 2023-06-30T10:41:46Z

extension/server/src/server.ts

+
+				} else {
+					// Path from home directory?
+					if (path.extname(cleanString) === ``) cleanString += `.*`;


This is not correct - you can't just take any file with the name as specified and a random suffix.

The rules are like this for copy-files specified without extension:

If the path does not end with a suffix (for example ".txt"), the compiler will search for the file as named, and also for files with suffixes of ".rpgle" or ".rpgleinc".

So three files may be picked:

File without suffix

File with suffix .rpgleinc

File with suffix .rpgle

We need to wait for 2.0.0 of vscode-ibmi to search for multiple extensions.

extension/server/src/server.ts

chrjorgensen · 2023-06-30T11:41:20Z

@worksofliam IMHO the RPG /COPY documentation is confusingly organized - the INCLUDE FILES page seems the most important. It was probably added when RPG source code was allowed in the IFS.

An example that I was not really aware of:
/COPY mylib/myfile,mymbr can mean

in the QSYS filesystem library mylib with source file myfile and member mymbr OR
in the IFS (root) filesystem subdirectory mylib in the current working directory with streamfile myfile,mymbr

The page above has more examples.

So both QSYS and IFS filesystem are searched by the compiler unless the filename is in quotes.

I won't even go into the IFS include path using INCDIR parameter or RPGINCDIR environment variable...!

chrjorgensen · 2023-06-30T12:42:44Z

One test still missing is /copy files in includePath - coming up later...

Signed-off-by: Liam Allan <mrliamallan@live.co.uk>

github-actions · 2023-06-30T14:35:38Z

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

worksofliam · 2023-06-30T14:39:01Z

✅ The copy of TESTPR225/SRCFIL,MBR3 no longer worked
- This is fixed.
✔️ The IFS copy of /dir1/dir2/file.rpg worked now.
❌ The IFS copy of /dir1/dir2/file worked - partially. Could only be resolved, when exact filename existed, but no seatch for suffixes .rpgle and .rpgleinc is performed.
- Left the current logic in until vscode-ibmi 2.0.0, which will allow us to search for many names.
✔️ The IFS copy of dir1/dir2/file.rpg worked now.
✅ I could not get the IFS copy of "ifs file containing blanks" to work.
- This is fixed. Share what your include directive looks like if it fails again.
✅ I could not get the IFS copy of 'ifs file containing blanks' to work.
- This is fixed. Share what your include directive looks like if it fails again.

extension/server/src/server.ts

github-actions · 2023-07-14T13:26:19Z

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

worksofliam · 2023-07-14T13:26:36Z

@chrjorgensen I know you're about to depart for holiday, but whenever you're back...

✅ The copy of TESTPR225/SRCFIL,MBR3 no longer worked
This is fixed.
✔️ The IFS copy of /dir1/dir2/file.rpg worked now.
✅ The IFS copy of /dir1/dir2/file worked - partially. Could only be resolved, when exact filename existed, but no seatch for suffixes .rpgle and .rpgleinc is performed.
Left the current logic in until vscode-ibmi 2.0.0, which will allow us to search for many names.
✔️ The IFS copy of dir1/dir2/file.rpg worked now.
✅ I could not get the IFS copy of "ifs file containing blanks" to work.
This is fixed. Share what your include directive looks like if it fails again.
✅ I could not get the IFS copy of 'ifs file containing blanks' to work.
This is fixed. Share what your include directive looks like if it fails again.

github-actions · 2023-07-17T20:31:44Z

👋 A new build is available for this PR based on 1ccde46.

github-actions · 2023-08-03T16:47:38Z

👋 A new build is available for this PR based on 2be46c7.

chrjorgensen

@worksofliam I made a new review of the source and include statements:

❓ The copy of MBR1 worked almost, member found was TESTPR225/QRPGLESRC/MBR1.rpgleinc - but member extension actually was TXT.
❓ The include of SRCFIL/MBR2 worked almost, member found was TESTPR225/SRCFIL/MBR2.rpgleinc - but member extension actually was RPGLE.
❓ The copy of TESTPR225/SRCFIL/MBR3 worked almost, member found was TESTPR225/SRCFIL/MBR3.rpgleinc - but member extension actually was RPGLE.
✔️ The IFS copy of /dir1/dir2/file.rpg worked fine.
❓ The IFS copy of /dir1/dir2/file only worked, when the filename was file - but not with file.rpgle or file.rpgleinc. Can we include multi file search now?
✔️ The IFS copy of dir1/dir2/file.rpg worked fine.
❌ The IFS copy of "ifs file containing blanks" could not be resolved, even though a file named ifs file containing blanks was put in the current working directory.
❌ The IFS copy of 'ifs file containing blanks' could not be resolved, even though a file named ifs file containing blanks was put in the current working directory.

I think we're very close to having it - only minor issues except for the question of multi file search... 👍

edit from barry: added numbers to make it easier to reference

worksofliam · 2023-08-11T00:05:27Z

Thanks for testing @chrjorgensen!

For 1, 2 & 3: Right now, when including a member we default to rpgleinc as the extension. Our APIs don't provide the extension (always simply returns .MBR) so I decided to use .rpgleinc for now. Adding an additional call to get the valid extension is expensive and didn't seem worthy for now. While this may confuse our developers, I think initially this is something we can start with.

For 5: I tested this and it seemed to work as you described:

For 7 & 8: I mentioned previously that this style of include (paths with spaces) won't be supported in this PR (a risk I am also willing to take). It looks like there is a bug in the memberResolve API that will need fixing before this can work. I don't want to hold up this PR. edit: I made a PR in vscode-ibmi and requested your review which, in theory, should solve this.

github-actions · 2023-08-11T00:06:41Z

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

chrjorgensen · 2023-08-11T11:35:16Z

@worksofliam IMHO it would be better to have no extension for members - since the extension is not specified in the /COPY statement and the extension actually is not part of the member name.

Instead of showing

it should just show /TESTPR225/QRPGLESRC/MBR1 - is this difficult to implement?

I have reviewed your PR #1482 and made a suggestion...

github-actions · 2023-08-11T14:03:35Z

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

chrjorgensen

@worksofliam I just did a final test: The hover is great now, 5 and 7-8 are still not working.

But we can fix this later, let's get this in now! Okay?

worksofliam · 2023-08-11T16:02:53Z

@chrjorgensen 5 isn't working for you? That is odd! I actually wonder if we should take a look at that together before we merge this.

worksofliam · 2023-08-11T19:07:42Z

@chrjorgensen I went ahead and fixed the issue where the path name was being cut off as you discovered! Whenever you have the time, take another look with my latest commit!

github-actions · 2023-08-11T19:07:54Z

👋 A new build is available for this PR based on 4bb0894.

chrjorgensen

@worksofliam Everything is working as expected now - even test 5, which was due to a mistake on my part!

Looking good! Approved! Merge at any time! 😄

worksofliam added 4 commits June 15, 2023 11:36

Member resolves now based on library list

db15a39

Signed-off-by: Liam Allan <mrliamallan@live.co.uk>

Improve performance of member resolving

816c297

Signed-off-by: Liam Allan <mrliamallan@live.co.uk>

Merge branch 'main' into feature/member_resolves

5713fda

Signed-off-by: Liam Allan <mrliamallan@live.co.uk>

Clearer code for member path parsing

021c717

Signed-off-by: Liam Allan <mrliamallan@live.co.uk>

Default to QRPGLESRC instead

bba721c

Signed-off-by: Liam Allan <mrliamallan@live.co.uk>

worksofliam requested review from sebjulliand and chrjorgensen June 23, 2023 20:34

worksofliam added 2 commits June 23, 2023 16:41

Restructure include resolution to be based on content

4dd7086

Signed-off-by: Liam Allan <mrliamallan@live.co.uk>

Support for local includePath resolving against the server

e1827a3

Signed-off-by: Liam Allan <mrliamallan@live.co.uk>

worksofliam changed the title ~~Feature/member resolves~~ Member resolves, local includePath support Jun 23, 2023

Merge branch 'main' into feature/member_resolves

23e5d62

Signed-off-by: Liam Allan <mrliamallan@live.co.uk>

worksofliam added the build Build in PR label Jun 23, 2023

chrjorgensen requested changes Jun 27, 2023

View reviewed changes

Clear file cache on close

6178f1d

Signed-off-by: Liam Allan <mrliamallan@live.co.uk>

Support for finding IFS files

173bfb7

Signed-off-by: Liam Allan <mrliamallan@live.co.uk>

worksofliam requested a review from chrjorgensen June 29, 2023 23:21

worksofliam changed the title ~~Member resolves, local includePath support~~ Member resolves, IFS resolves and local includePath support Jun 29, 2023

Support quotes or double quotes

7dfe7f7

Signed-off-by: Liam Allan <mrliamallan@live.co.uk>

chrjorgensen requested changes Jun 30, 2023

View reviewed changes

Fix issue of unix and member name clash

a11c031

Signed-off-by: Liam Allan <mrliamallan@live.co.uk>

chrjorgensen reviewed Jun 30, 2023

View reviewed changes

extension/server/src/server.ts Show resolved Hide resolved

Support multi-extension lookup for IFS files

f912aed

worksofliam requested a review from chrjorgensen July 14, 2023 13:25

worksofliam linked an issue Jul 17, 2023 that may be closed by this pull request

COPY/INCLUDE doesn't handle library list #127

Closed

Merge branch 'main' into feature/member_resolves

1ccde46

Merge branch 'main' into feature/member_resolves

2be46c7

chrjorgensen requested changes Aug 7, 2023

View reviewed changes

Update types and audit

d8f74bc

Display better name for member includes

4920529

chrjorgensen self-requested a review August 11, 2023 16:00

chrjorgensen approved these changes Aug 11, 2023

View reviewed changes

worksofliam added 2 commits August 11, 2023 14:55

Merge branch 'main' into feature/member_resolves

fb49be3

Fix issue with resolving include paths

4bb0894

worksofliam requested a review from chrjorgensen August 14, 2023 16:06

chrjorgensen approved these changes Aug 14, 2023

View reviewed changes

worksofliam merged commit 8555858 into main Aug 14, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Member resolves, IFS resolves and local `includePath` support #225

Member resolves, IFS resolves and local `includePath` support #225

worksofliam commented Jun 15, 2023 •

edited

Loading

chrjorgensen commented Jun 18, 2023 •

edited

Loading

worksofliam commented Jun 18, 2023

worksofliam commented Jun 23, 2023

worksofliam commented Jun 23, 2023 •

edited

Loading

chrjorgensen left a comment

github-actions bot commented Jun 29, 2023

github-actions bot commented Jun 29, 2023

worksofliam commented Jun 29, 2023 •

edited

Loading

worksofliam commented Jun 29, 2023

github-actions bot commented Jun 30, 2023

chrjorgensen left a comment

chrjorgensen Jun 30, 2023

worksofliam Jun 30, 2023

chrjorgensen commented Jun 30, 2023

chrjorgensen commented Jun 30, 2023

github-actions bot commented Jun 30, 2023

worksofliam commented Jun 30, 2023

github-actions bot commented Jul 14, 2023

worksofliam commented Jul 14, 2023

github-actions bot commented Jul 17, 2023

github-actions bot commented Aug 3, 2023

chrjorgensen left a comment •

edited by worksofliam

Loading

worksofliam commented Aug 11, 2023 •

edited

Loading

github-actions bot commented Aug 11, 2023

chrjorgensen commented Aug 11, 2023

github-actions bot commented Aug 11, 2023

chrjorgensen left a comment

worksofliam commented Aug 11, 2023

worksofliam commented Aug 11, 2023 •

edited

Loading

github-actions bot commented Aug 11, 2023

chrjorgensen left a comment •

edited

Loading

Member resolves, IFS resolves and local includePath support #225

Member resolves, IFS resolves and local includePath support #225

Conversation

worksofliam commented Jun 15, 2023 • edited Loading

Changes

Checklist

chrjorgensen commented Jun 18, 2023 • edited Loading

worksofliam commented Jun 18, 2023

worksofliam commented Jun 23, 2023

worksofliam commented Jun 23, 2023 • edited Loading

chrjorgensen left a comment

Choose a reason for hiding this comment

github-actions bot commented Jun 29, 2023

github-actions bot commented Jun 29, 2023

worksofliam commented Jun 29, 2023 • edited Loading

worksofliam commented Jun 29, 2023

github-actions bot commented Jun 30, 2023

chrjorgensen left a comment

Choose a reason for hiding this comment

chrjorgensen Jun 30, 2023

Choose a reason for hiding this comment

worksofliam Jun 30, 2023

Choose a reason for hiding this comment

chrjorgensen commented Jun 30, 2023

chrjorgensen commented Jun 30, 2023

github-actions bot commented Jun 30, 2023

worksofliam commented Jun 30, 2023

github-actions bot commented Jul 14, 2023

worksofliam commented Jul 14, 2023

github-actions bot commented Jul 17, 2023

github-actions bot commented Aug 3, 2023

chrjorgensen left a comment • edited by worksofliam Loading

Choose a reason for hiding this comment

worksofliam commented Aug 11, 2023 • edited Loading

github-actions bot commented Aug 11, 2023

chrjorgensen commented Aug 11, 2023

github-actions bot commented Aug 11, 2023

chrjorgensen left a comment

Choose a reason for hiding this comment

worksofliam commented Aug 11, 2023

worksofliam commented Aug 11, 2023 • edited Loading

github-actions bot commented Aug 11, 2023

chrjorgensen left a comment • edited Loading

Choose a reason for hiding this comment

Member resolves, IFS resolves and local `includePath` support #225

Member resolves, IFS resolves and local `includePath` support #225

worksofliam commented Jun 15, 2023 •

edited

Loading

chrjorgensen commented Jun 18, 2023 •

edited

Loading

worksofliam commented Jun 23, 2023 •

edited

Loading

worksofliam commented Jun 29, 2023 •

edited

Loading

chrjorgensen left a comment •

edited by worksofliam

Loading

worksofliam commented Aug 11, 2023 •

edited

Loading

worksofliam commented Aug 11, 2023 •

edited

Loading

chrjorgensen left a comment •

edited

Loading