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.

Sign up for GitHub

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

Jump to bottom

Generate binder source from module or service program #2485

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Generate binder source from module or service program #2485

wants to merge 11 commits into from

Conversation

janfh
Copy link
Contributor

@janfh janfh commented Jan 25, 2025 •
edited
Loading

Fixes #2484

Changes

  • New command and object browser option to generate binder source from module or service program
  • New functions in IBMiContent for retrieving module exports using DSPMOD *OUTFILE and service program exports using the PROGRAM_EXPORT_IMPORT_INFO view
  • Generates binder source in a new editor

How to test this PR

Examples:

  1. Locate a module and a service program in object browser
  2. Right click and select "Generate binder source"
  3. Verify binder source generated in editor

Checklist

  • have tested my change
  • have created one or more test cases
  • updated relevant documentation

@janfh janfh changed the title Feature/binder source Generate binder source from module or service program Jan 25, 2025
@worksofliam
Copy link
Contributor

Honestly I love the idea!

There is some overlap with the FS extension (look up vscode-ibmi-fs on our org), but since it hasn't been released yet and isn't anywhere near being released, I believe it could live here until then.

Thoughts @sebjulliand?

@worksofliam
Copy link
Contributor

@janfh What are the chances you can write new test cases for the new APIs you added? Perhaps against objects in QSYS2?

@janfh
Copy link
Contributor Author

janfh commented Jan 27, 2025

@janfh What are the chances you can write new test cases for the new APIs you added? Perhaps against objects in QSYS2?

Sure :) But I'm not sure if there is any *MODULE to to test "getModuleExport". Do you know of any?

@worksofliam
Copy link
Contributor

@janfh You're right, I don't know of many modules to test with in either QSYS, QSYS2, or SYSTOOLS. If you have to skip one API, that's ok. But if you're feeling adventurous, perhaps your test could create a module with a simple export.

@janfh
Copy link
Contributor Author

janfh commented Jan 27, 2025

@worksofliam : I have added a test for each of the two new methods in IBMiContent

worksofliam
worksofliam approved these changes Jan 28, 2025
View reviewed changes
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.

Overall a nice change, but sadly I have a test case not passing and also am making a recommendation to the parameter type on your new method.

src/ui/views/objectBrowser.ts Show resolved Hide resolved
src/api/tests/suites/content.test.ts Outdated Show resolved Hide resolved
src/api/tests/suites/content.test.ts Outdated Show resolved Hide resolved
worksofliam
worksofliam requested changes Jan 28, 2025
View reviewed changes
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.

Sorry - meant to press request changes.

@janfh
Copy link
Contributor Author

janfh commented Jan 28, 2025

Sorry - meant to press request changes.

Thanks for the feedback :)

@janfh janfh requested a review from worksofliam January 28, 2025 16:59
sebjulliand
sebjulliand requested changes Feb 2, 2025
View reviewed changes
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.

Nice feature, thanks @janfh !
I just have a few style/simplification suggestions related to the code. Thanks!

src/api/tests/suites/content.test.ts Outdated Show resolved Hide resolved
src/api/tests/suites/content.test.ts Outdated Show resolved Hide resolved
src/api/tests/suites/content.test.ts Outdated Show resolved Hide resolved
src/api/tests/suites/content.test.ts Outdated Show resolved Hide resolved
package.json
},
{
"command": "code-for-ibmi.generateBinderSource",
"when": "view == objectBrowser && viewItem =~ /^object.(module|srvpgm).*/",
Copy link
Collaborator

Choose a reason for hiding this comment

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

You may want to escape the dot here, since you'll be looking at an actual dot (and not any character).

Suggested change
"when": "view == objectBrowser && viewItem =~ /^object.(module|srvpgm).*/",
"when": "view == objectBrowser && viewItem =~ /^object\.(module|srvpgm).*/",

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you sure? The suggested change seems to be invalid syntax? Or am I missing something else here?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, right, that would be object\\.; the slash has to be escaped.

src/api/IBMiContent.ts Outdated Show resolved Hide resolved
src/api/IBMiContent.ts Outdated Show resolved Hide resolved
src/api/types.ts Outdated Show resolved Hide resolved
@sebjulliand
Copy link
Collaborator

Honestly I love the idea!

There is some overlap with the FS extension (look up vscode-ibmi-fs on our org), but since it hasn't been released yet and isn't anywhere near being released, I believe it could live here until then.

Thoughts @sebjulliand?

We'll definitely have to keep in mind to move it to FS in due time, since it's really an action targeted at objects 😄

@janfh janfh had a problem deploying to testing_environment February 3, 2025 16:04 — with GitHub Actions Failure
@janfh janfh temporarily deployed to testing_environment February 5, 2025 19:18 — with GitHub Actions Inactive
@janfh janfh temporarily deployed to testing_environment February 5, 2025 20:49 — with GitHub Actions Inactive
@janfh
Copy link
Contributor Author

janfh commented Feb 5, 2025

Nice feature, thanks @janfh ! I just have a few style/simplification suggestions related to the code. Thanks!

Thanks for the feedback! And sorry for the slow response :) Hope you will have another look at this

@janfh janfh requested a review from sebjulliand February 5, 2025 20:58
sebjulliand
sebjulliand approved these changes Feb 12, 2025
View reviewed changes
Copy link
Collaborator

@sebjulliand sebjulliand left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Lovely! Looks good to me!
No worries, it's been busy around here too as you could see!

@sebjulliand
Copy link
Collaborator

@worksofliam it's fine by me. You can have a final look and merge!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sebjulliand sebjulliand sebjulliand approved these changes

@worksofliam worksofliam Awaiting requested review from worksofliam

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Generate binder source from module
3 participants
@janfh @worksofliam @sebjulliand