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

(URI data value) fixes spaces assumed as underscores also for non-wiki URIs #5396

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

thomas-topway-it
Copy link
Contributor

@thomas-topway-it thomas-topway-it commented Jan 13, 2023

fixes issue/4910 and partial revert of #2201

fixes underlying problem causing SemanticMediaWiki/SemanticResultFormats#93 (solved in a recent commit of SemanticResultFormats with a cpu-consuming work-around)

As in the title, the commit #2201 assumes that all spaces (%20) in the URI data values correspond to underscore symbols, which is semantically true only for WIKI's URIs, either external or internal. (for this reason, legitimate URIs including spaces, not exchangeable with underscores, cease to work once imported in SMW)

This PR tries to address this issue, however the impact of the changes should be evaluated against the methods getShortWikiText, getShortHTMLText, getLongWikiText and getLongHTMLText (includes/datavalues/SMW_DV_URI.php)

Also note that the corresponding tests have been identified but not yet updated because they imply a little design change however affecting many tests: see for instance the image below which is related to the new output of getShortWikiText (includes/datavalues/SMW_DV_URI.php)

uri-test-problem

(in the new output, the space, if any, is not anymore converted to an underscore, and therefore the caption must be explicitly declared in the wikitext, otherwise the url gets broken in two parts, one of them considered the caption -- this is correct, however other reviewers might evaluate if the new output is at the same time appropriate for the different uses of the method getShortWikiText)

@thomas-topway-it
Copy link
Contributor Author

thomas-topway-it commented Jan 27, 2023

Ideally, before approving the Pull request, a key SMW developer is expected to verify that the new output produced by the changes of the SMWURIValue -> getShortWikiText method (as mentioned above) is acceptable and consistent with all use-cases throughout SMW of the same method. (evaluating at the same time the expected and actual results of tests)
(the new output assumes that the caption cannot anymore be used to denote a wikitext url, because it could now contain a space and the space itself is used in wikitext url to distinguish the url from the caption: for this reason, both the url and the caption have to be explicitly declared also in the SMWURIValue -> getShortWikiText method)

@thomas-topway-it
Copy link
Contributor Author

please also check the thread here #4910

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.

unwanted underscores in url property value
1 participant