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

Update Jelly Tag Library XSDs #149

Merged
merged 12 commits into from
Apr 26, 2023
Merged

Conversation

NotMyFault
Copy link
Member

These updates have been generated by running mvn clean org.jvnet.maven-jellydoc-plugin:maven-jellydoc-plugin:1.11:jellydoc in the jelly and stapler repository. The output aligns with the format we're using there and the binaries produced.

To note, there are more XSDs available in the https://github.com/jenkinsci/jelly/tree/master/jelly-tags folder, but we don't build or distribute these, and they are not maven-ized. Therefore, I didn't include XSD updates for them.

Closes #145
Fixes #92

@duemir
Copy link
Member

duemir commented Apr 3, 2023

Curiously I don't see <xsd:element name="out"> 🤔

@NotMyFault
Copy link
Member Author

NotMyFault commented Apr 3, 2023

Curiously I don't see <xsd:element name="out"> 🤔

A couple of classes register the ExprTag:

The "expr" takes precedence, first come first served rules, I assume. The javadoc comments are registered under this tag name.

That's the only tag that's registered twice, no idea why it's been done like that.

I see we use j:expr a handful number of times in core and plugins: https://github.com/search?q=org%3Ajenkinsci+lang%3Axml+%22j%3Aexpr%22&type=code
I'd propose I migrate these to j:out, remove the assignment of "expr" for the ExprTag in Jelly, given j:out is what is used by the vast majority, and return to #145 at a final point, once this done, thoughts?
This should unblock merging this PR.

@duemir
Copy link
Member

duemir commented Apr 5, 2023

Ideally, the IntelliJ plugin reads the schema directly from the Java files that implement the tags, but it is a sizable chunk of work. XSDs are an expedient way of adding support. I can merge this, duplicate the expr tag, and rename the duplicate to out to get a good approximation of the tag library. Jelly doesn't change that much so it should be fine.

Regarding removing expr tag, I don't have an opinion. I mostly read plugin source code. I only write a tiny little bit occasionally.

@duemir duemir self-requested a review April 5, 2023 06:12
@NotMyFault
Copy link
Member Author

Sounds good!

@duemir duemir changed the title Update XSDs Update Jelly Tag Library XSDs Apr 25, 2023
duemir added 2 commits April 25, 2023 17:38
These are Tag classes that are registered multiple times.
It was missing because it is implemented as a TagScript
Copy link
Member

@duemir duemir left a comment

Choose a reason for hiding this comment

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

A handful of XSDs had an invalid, empty element definition <xsd:element name="">
A surprising number of Stapler tags were not previously in the schema.
Two that are still there were removed (st:documentation, st:once). I am guessing they were added by hand. There was a third TagScript tag that is not handled by a generator that was missing st:getOutput. I added them all back.
I think XSDs look decent now.

@duemir duemir merged commit 2402c41 into jenkinsci:master Apr 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Jelly Core Schema is not complete
2 participants