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

Look for member on SYSBAS if config.sourceASP is set #1651

Merged
merged 2 commits into from
Nov 18, 2023

Conversation

sebjulliand
Copy link
Collaborator

Changes

This fixes an issue that can happen when downloadMemberContent is called to open a member located on *SYSBAS and an iASP is specified in the Connection Settings (i.e. config.sourceASP).

When config.sourceASP is set, downloadMemberContent will always assume the member content to fetch is located on that IASP. If the member is actually on *SYSBAS, it can never be read.

This PR makes downloadMemberContent tries twice to fetch a member if an IASP is set in the settings:

  • First it assumes the member is on the IASP (as it used to)
  • If not found, it will try again without the IASP in the path
  • If still not found, then it crashes

Checklist

  • have tested my change

Signed-off-by: Seb Julliand <sjulliand@arcadsoftware.com>
@sebjulliand sebjulliand added the bug A confirmed issue when something isn't working as intended label Nov 15, 2023
@sebjulliand sebjulliand requested a review from a team November 15, 2023 15:58
@worksofliam worksofliam self-assigned this Nov 15, 2023
@worksofliam
Copy link
Contributor

worksofliam commented Nov 15, 2023

Nice. Having a look at the code, it looks like it's falling back to *SYSBAS if it's not in the ASP the user defines first, so it looks in this order:

  1. If the user has an ASP defined, look there. Return it if found.
  2. Otherwise, look in *SYSBAS (undefined in the asp variable).

Just about to give it a test.

Copy link
Contributor

@worksofliam worksofliam left a comment

Choose a reason for hiding this comment

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

Looks like it worked for reading files.. but not saving!

I don't have any ASPs to test with, but it did allow me to test with an invalid name to test the fall back:

image

In the output:

.: system "CPYTOSTMF FROMMBR('/boop/QSYS.lib/LIAMA.lib/QRPGLESRC.file/JP.mbr') TOSTMF('/tmp/vscodetemp-O_3z3bFCsE') STMFOPT(*REPLACE) STMFCCSID(1208) DBFCCSID(*FILE)"
{
    "code": 255,
    "signal": null,
    "stdout": "",
    "stderr": "CPFA0A9: Object not found.  Object is /boop/QSYS.lib/LIAMA.lib/QRPGLESRC.file/JP.mbr.\nCPFA097: Object not copied.  Object is /boop/QSYS.lib/LIAMA.lib/QRPGLESRC.file/JP.mbr."
}

.: system "CPYTOSTMF FROMMBR('/QSYS.lib/LIAMA.lib/QRPGLESRC.file/JP.mbr') TOSTMF('/tmp/vscodetemp-O_yubtaBfe') STMFOPT(*REPLACE) STMFCCSID(1208) DBFCCSID(*FILE)"
{
    "code": 0,
    "signal": null,
    "stdout": "CPCA082: Object copied.",
    "stderr": ""
}

When trying to save this member:

image

Signed-off-by: Seb Julliand <sebjulliand@gmail.com>
@sebjulliand
Copy link
Collaborator Author

sebjulliand commented Nov 15, 2023

Ugh, yeah, I forgot that there is a download AND an upload memberContent 😅
I fixed the upload too. Go for it!

@worksofliam worksofliam added this to the 2.5.0 release milestone Nov 17, 2023
Copy link
Contributor

@worksofliam worksofliam left a comment

Choose a reason for hiding this comment

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

Yep, now it works! This is a totally nice UX improvement.

@sebjulliand Merge when you are ready!!

@sebjulliand sebjulliand merged commit ec8a4dc into master Nov 18, 2023
@sebjulliand sebjulliand deleted the fix/sourceASPonSysbas branch November 18, 2023 18:24
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
Development

Successfully merging this pull request may close these issues.

2 participants