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

Feature/rpgle_2023_oct #265

Merged
merged 11 commits into from
Oct 20, 2023
Merged

Feature/rpgle_2023_oct #265

merged 11 commits into from
Oct 20, 2023

Conversation

worksofliam
Copy link
Contributor

Changes

  • Token support for WHEN-IS and WHEN-IN
  • Token support for enum structures groups.
  • Enum structure groups appear in content assist and outline view

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.

Copy link
Contributor

@chrjorgensen chrjorgensen left a comment

Choose a reason for hiding this comment

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

@worksofliam Looking good, the ENUMs and constants are recognized and shown in Outline and Content Assist. 👍

I have only a few issues:

  • How do I test the WHEN tokens being recognized?
  • I would expect the subitems in a qualified ENUM to be shown in Content Assist, when I type the name of the ENUM and a dot and Ctrl-Space - but they are not in the list? This is also a problem for subfields in a data structure, so maybe it is an old problem? If I don't qualify the ENUM or data structure, the subitems appear in the Content Assist list...

@worksofliam
Copy link
Contributor Author

@chrjorgensen

How do I test the WHEN tokens being recognized?

The way we can test for this is by using WHEN, WHEN-IN or WHEN-IS with the indent lint rule enabled.

when I type the name of the ENUM and a dot and Ctrl-Space - but they are not in the list?

That is odd. See this screenshot below that shows the content assist after I use the dot/period trigger.

image

@worksofliam worksofliam mentioned this pull request Oct 14, 2023
11 tasks
@worksofliam
Copy link
Contributor Author

@chrjorgensen I added two tests cases for the new WHEN operators. Thanks for getting on my case to do that!

@worksofliam worksofliam linked an issue Oct 15, 2023 that may be closed by this pull request
@chrjorgensen
Copy link
Contributor

@worksofliam I did a fresh install of VS Code, Code for IBM i and RPGLE extension in a new Windows profile.
But I get this when opening a slightly modified sample of your code and pressing Ctrl-Enter after the dot:

Screenshot 2023-10-17 222429

I don't know what going on here... not like your experience at all...???

@worksofliam
Copy link
Contributor Author

@chrjorgensen okay I see what's going on.

Control+space will only show in scope references. You will only see subfields when you press dot after the structure.

@chrjorgensen
Copy link
Contributor

@worksofliam Finally found the setting, that prohibited the pop-up when entering the dot after the structure:

"editor.suggestOnTriggerCharacters": false,

After removal of this setting, the subfields are shown after entering the dot character.

Will review your code tomorrow.

@worksofliam
Copy link
Contributor Author

Totally cool! Weird that it was off though.

@chrjorgensen
Copy link
Contributor

Totally cool! Weird that it was off though.

Probably me that have turned if off at some point - annoyed by the pop-ups... 😆

Copy link
Contributor

@chrjorgensen chrjorgensen left a comment

Choose a reason for hiding this comment

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

@worksofliam Looking good - only a few issues...

language/linter.ts Show resolved Hide resolved
language/tokens.ts Show resolved Hide resolved
@worksofliam worksofliam self-assigned this Oct 18, 2023
@chrjorgensen chrjorgensen self-requested a review October 19, 2023 21:13
language/tokens.ts Outdated Show resolved Hide resolved
tests/suite/linter.js Outdated Show resolved Hide resolved
Copy link
Contributor

@chrjorgensen chrjorgensen left a comment

Choose a reason for hiding this comment

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

@worksofliam Some final issues - when fixed we can approve and merge! 🚀

Co-authored-by: Christian Jorgensen <[email protected]>
@worksofliam
Copy link
Contributor Author

@chrjorgensen Issues resolved. Over to you for approval, and if you want to.. merge 😄

Copy link
Contributor

@chrjorgensen chrjorgensen left a comment

Choose a reason for hiding this comment

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

@worksofliam Great - let's approve and merge! 🚀

@chrjorgensen chrjorgensen self-requested a review October 20, 2023 16:31
@chrjorgensen
Copy link
Contributor

@worksofliam I don't have write access and can't merge... 😭

image

@worksofliam worksofliam merged commit 09e0df5 into main Oct 20, 2023
1 check passed
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.

New op code ON-EXCP needs to have same indent as ON-ERROR
2 participants