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

TriGParser: distinguish empty entities from no-entity being read #42

Merged
merged 4 commits into from
Feb 28, 2024

Conversation

zozlak
Copy link
Contributor

@zozlak zozlak commented Feb 27, 2024

Solves the #37 by throwing an informative exception describing why the parsing can not be done without the document base IRI being set.

@zozlak
Copy link
Contributor Author

zozlak commented Feb 27, 2024

See #37 (comment)

What is your (@k00ni, @pietercolpaert ) preferred way to deal with that?

I can either adjust the parser to consistently return empty subject/predicate/object on "diamonds" (<>) without the base IRI set (it is only a matter of turning comparisons like this one into type-strict ones) or I can adjust the failing test to expect the "please set the documentIRI" exception.

@pietercolpaert
Copy link
Owner

Adjusting the test to expect an exception takes my preference

* removed the prefixed-only IRIs input line from the first test scenario
  as this does not belong to the testBlankNodes() tests and is tested
  aleady in testIssue37()
* turned the empty prefixed IRIs test scenario into two - first, where
  and error is expected due to unknown document base IRI and second,
  where parsing succeeds thanks to `documentIRI` parser option being set
@zozlak zozlak changed the title TriGParser: distinguish empty entities from no-etity being read TriGParser: distinguish empty entities from no-entity being read Feb 28, 2024
Copy link
Collaborator

@k00ni k00ni left a comment

Choose a reason for hiding this comment

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

👍 Thank you @zozlak, looks good to me, just a few remarks.

I also support @pietercolpaert's preference.

Off topic: The current coding style should be addressed in the future, because it makes code (especially the tests) very hard to read. Therefore only a few remarks in this regard.

test/TriGParserTest.php Outdated Show resolved Hide resolved
test/TriGParserTest.php Outdated Show resolved Hide resolved
@k00ni k00ni merged commit 01bf556 into pietercolpaert:master Feb 28, 2024
1 check passed
@zozlak zozlak deleted the issue37 branch June 25, 2024 12:09
@zozlak zozlak restored the issue37 branch June 25, 2024 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Turtle parser fails for unknown reason (SPARQL 1.1 Syntax Update 1 manifest.ttl)
3 participants