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

Attribute manipulation should be syntax-checked #136

Open
baszoetekouw opened this issue Sep 12, 2024 · 5 comments
Open

Attribute manipulation should be syntax-checked #136

baszoetekouw opened this issue Sep 12, 2024 · 5 comments

Comments

@baszoetekouw
Copy link
Member

To catch obvious mistakes, it would be nice if we could syntax-check the AM PHP code before it can be submitted.
Also adding syntax-coloring would help to catch problems.

@thijskh
Copy link
Member

thijskh commented Oct 8, 2024

This is a bit of a challenge since Manage is obviously not PHP.

One, slightly crude approach could be that EB syntax checks any incoming manipulation on PUSH, and refuses to update when this has a syntax error

@pmeulen
Copy link
Member

pmeulen commented Oct 8, 2024

There is no native PHP way to syntax check an AM AFIK. You either eval / include the php code or you invoke the php binary with the -l option.
A client side syntax hi-lighting exists (e.g. highlight.js).
I think a syntax hi-light might already help a lot with catching syntax errors.
In the end the idea is minimalise th use attribute manipulations.

@tvdijen
Copy link
Contributor

tvdijen commented Oct 8, 2024

There are ways to do something with the result of highlight.js:
https://highlightjs.readthedocs.io/en/latest/plugin-recipes.html

Could make the save-button greyed-out if the result indicates a syntax error?

@thijskh
Copy link
Member

thijskh commented Oct 8, 2024

I've tested it and this works to test any AM code for parse errors:

try {
    @eval($am);
} catch (ParseError $e) {
    echo "Dat ging dus mis. Want: " . $e->getMessage();
    exit(1);
}

The monkey tail is there to avoid notices and warnings because the needed variables for an AM to function are not set.

@baszoetekouw
Copy link
Member Author

We'll implement a chck in EB on import, see OpenConext/OpenConext-engineblock#1778

I would still like to a bit more editing functionality in the AM pane in Manage though. Would it be possible to add syntax highlighting there, as suggested by @pmeulen en @tvdijen ?

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

No branches or pull requests

4 participants