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

ResultFieldMatchFinder / Support |+lang= filter in PrintRequests #2037

Merged
merged 1 commit into from Nov 26, 2016

Conversation

@mwjames
Copy link
Contributor

commented Nov 20, 2016

This PR is made in reference to: #1344, #594

This PR addresses or contains:

  • |+lang= is to support filtering of results that match a specific language and is applied during query post-processing after the condition has retrieved matchable entities. (The PrintRequest and hereby the DV may match text in different languages and unless the query condition was designed in a way that would made them reducible, it would display them all)
  • Using the post-filtering mechanism allows that a query condition can be indifferent towards possible language matches, yet enables to only select the one that match the denoted lang value
  • |+lang= filtering is only applied to instances of type Monolingual text

This PR includes:

  • Tests (unit/integration)
  • CI build passed
@mwjames mwjames added the enhancement label Nov 20, 2016
@mwjames mwjames added this to the SMW 2.5 milestone Nov 20, 2016
@mwjames

This comment has been minimized.

Copy link
Contributor Author

commented Nov 20, 2016

@kghbln FYI

{{#ask: [[Has alternative label::+]]
 |?Has alternative label|+lang=en
}}

http://sandbox.semantic-mediawiki.org/wiki/Issue/2037

@kghbln

This comment has been minimized.

Copy link
Member

commented Nov 20, 2016

|+lang= filtering is only applied to instances of type Monolingual text

Does this also make sense, though in a reduced degree to "Has property description"? like e.g. here on sandbox

Going further I think it will be nice to use this feature it in conjunction with a variable which is providing the language code from the users preference setting. I will have a look if there is an extension for this around.

@kghbln

This comment has been minimized.

Copy link
Member

commented Nov 20, 2016

I will have a look if there is an extension for this around.

Great, found it: MyVariables

@mwjames

This comment has been minimized.

Copy link
Contributor Author

commented Nov 20, 2016

Does this also make sense, though in a reduced degree to "Has property description"?

Lucky us, Has property description and Has preferred property label are defined as Monolingual text type hence |+lang= can be applied here as well.

image

@kghbln

This comment has been minimized.

Copy link
Member

commented Nov 20, 2016

Lucky us, Has property description and Has preferred property label are defined as Monolingual text type hence |+lang= can be applied here as well.

I have the feeling that this does not just work by chance. ;)

Great, found it: MyVariables

Now also available on sandbox.

@mwjames mwjames merged commit 42a829d into master Nov 26, 2016
3 checks passed
3 checks passed
Scrutinizer 5 updated code elements
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@mwjames mwjames deleted the lang-res branch Nov 26, 2016
@mwjames mwjames referenced this pull request Nov 26, 2016
2 of 2 tasks complete
@kghbln

This comment has been minimized.

Copy link
Member

commented Nov 26, 2016

@mwjames

Hmm, I did a field test with "Has property description" While it works for "fr" i.e. showing nothing is only works for the first property "Foaf:homepage" and not for "Foaf:knows" or "Foaf:name" when using "en" and "de". The latter two show both languages.

@mwjames

This comment has been minimized.

Copy link
Contributor Author

commented Nov 26, 2016

... not for "Foaf:knows" or "Foaf:name" when using "en" and "de". The latter two show both languages.

This happens when you only test half a meal and declare it as delicious. Should be fixed with #2048 containing a similar test case.

@kghbln

This comment has been minimized.

Copy link
Member

commented Nov 26, 2016

Should be fixed with #2048 containing a similar test case.

It works! Perfect! 👍

@mwjames mwjames referenced this pull request Apr 15, 2017
2 of 2 tasks complete
@kghbln

This comment has been minimized.

Copy link
Member

commented Jun 24, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.