Github user osma commented on the pull request:
https://github.com/apache/jena/pull/64#issuecomment-103646019
Excellent work! Looks almost ready for merging.
A couple of comments:
1.You say that "?s text:query (rdfs:label 'word' 'lang:none') will target
unlocalized literals". But the code does this:
```java
String qs2 = !"none".equals(langArg)?
field + ":" + langArg : "-" + field + ":*";
```
In my understanding, this will do a wildcard search on the language field
if the query argument is "lang:none". So it wouldn't be possible to search for
non-language-tagged literals. Unless I'm missing something, I'd replace the
above expression with simply this line:
```java
String qs2 = field + ":" + langArg;
```
2.You have now removed some of the old constructors from EntityDefinition
and the create* methods from TextDatasetFactory (and had to change the example
code and some unit tests accordingly). While this leads to cleaner code, it
also breaks the API so old code has to be changed. This might actually be a
good moment to do it, as Jena has just bumped the version number to 3 and other
APIs are also being changed, but I think we need opinions on that. If it turns
out to be too disruptive, then compatibility methods could be added back.
Could you write a message to the dev@ list and explain your contribution? I
could then join in with comments in favor of merging.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
