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/surround with monitor action #223

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

sebjulliand
Copy link
Contributor

Changes

Inspired by this discussion: https://github.com/orgs/halcyon-tech/discussions/1345

Added an RPGLE submenu and a Surround with monitor action for rpgle editors.

Surrounding will be allowed only for Free RPGLE code. Fixed RPG or mixed RPG will show a warning.
The action will surround the code, based on:

  • The current cursor position
    Code_mrfBFQSxCF

  • The current selection
    Code_C1HBThwqtb

It's not necessary to select the entire code. The action will select the whole line(s).

Checklist

  • have tested my change
  • for feature PRs: PR only includes one feature enhancement.

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.

A great change! I really like what we could do this with.. I have some wild ideas for adding more. Love the idea that they're bound to commands. I imagine have shortcuts to surround with a certain block!

return true;
}
else {
for (let i = currentLine; i < document.lineCount; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure about this check. In modern versions of RPGLE (7.3+), /free and /end-free are optional, so if they're not used, then it might always return false by the looks of it.

if (editor) {
const document = editor.document;
let line = editor.selection.start.line;
if (isFreeRPGLE(document, line)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This logic is interesting, checking if the logic is currently free format. Perhaps instead if the document is not **free, substring the first 8(?) characters and then surround that? That might be nicer than checking for /free and /end-free.

@sebjulliand
Copy link
Contributor Author

Pretty good feedback @worksofliam , thanks!
Here's an alternative: we don't rely on /free and /end-free and try to detect if it's actually Free when the program itself is not Fully Free. What's your thought?

@worksofliam
Copy link
Contributor

@sebjulliand

This is a truly late reply, but...

Here's an alternative: we don't rely on /free and /end-free and try to detect if it's actually Free when the program itself is not Fully Free. What's your thought?

Because /FREE and /END-FREE are optional directives. We can't rely on them to detect if it's free format or not. Sad times.

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.

None yet

2 participants