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

RDF Formatter Contribution #2261

Merged
merged 15 commits into from
Sep 27, 2024

Conversation

fkleedorfer
Copy link
Contributor

@fkleedorfer fkleedorfer commented Sep 16, 2024

Adds a formatter for RDF

In its current form, only the TTL format can be formatted using turtle-formatter. All other formats could be handled with Jena, but are currently not implemented.

The PR contains code for a maven plugin, but not a gradle/sbt plugin.

Options are:

  • failOnWarnings: fail if the Jena parser warns
  • verify: after formatting, parse both input and formatted output to a jena Model and compares them. If they are not isomorphic, outputs an error message
  • turtle: a list of style options for the turtle-formatter as documented in the turtle-formatter README Not all options are supported, though.

The PR includes smoke tests that use input/output folders containing files with identical names, for which a utility method was added to the ResourceHarness.

@fkleedorfer
Copy link
Contributor Author

fkleedorfer commented Sep 16, 2024

FYI: Expecting a dependency update for turtle-formatter soon-ish (PR already submitted). If it is allowed by your review process, I'd bump that dependency as it becomes available.

EDIT: that's done.

@nedtwigg
Copy link
Member

Looks great! Needs an entry in the plugin-maven/README.md docs and it's good to merge (assuming CI passes)

Comparing graph nodes using their `equals()` method is used under the hood by the isomorphicity check and therefore has to be used in the diff calcluation as well (otherwise the latter might not find any differences).

Special handling of blank nodes has been omitted as it is hard to find which ones are different and which ones are not, so we just print them all for now.
@fkleedorfer
Copy link
Contributor Author

I think it's done now.

@fkleedorfer
Copy link
Contributor Author

@nedtwigg ... what do I do? rebase and force-push ?

@nedtwigg
Copy link
Member

Nope, I just fixed the merge conflict, if CI passes this should be good to go.

@nedtwigg
Copy link
Member

Run ./gradlew spotbugsMain and I think it'll show you the problem.

@fkleedorfer
Copy link
Contributor Author

Done.

@fkleedorfer
Copy link
Contributor Author

Limited the tests to JDK >= 17 to deal with the tests failing for java 11. Hope that's it now.

@fkleedorfer
Copy link
Contributor Author

@nedtwigg could you please approve the workflow runs? Thanks!

@nedtwigg nedtwigg merged commit dbe72d8 into diffplug:main Sep 27, 2024
3 checks passed
@fkleedorfer fkleedorfer mentioned this pull request Oct 4, 2024
@nedtwigg
Copy link
Member

Released in plugin-gradle 7.0.0.BETA3 and plugin-maven 2.44.0.BETA3.

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.

2 participants