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

ListResultPrinter / Fix usage of sep in format=template #2022

Merged
merged 1 commit into from Nov 19, 2016
Merged

Conversation

mwjames
Copy link
Contributor

@mwjames mwjames commented Nov 19, 2016

This PR is made in reference to: # [0, 1]

This PR addresses or contains:

  • Most likely introduced with 66b7761

This PR includes:

  • Tests (unit/integration)
  • CI build passed

[0] Issues with the "sep" function in mode Query template.
[1] http://naruto.wikia.com/wiki/Thread:217742#29

@mwjames mwjames added this to the SMW 2.5 milestone Nov 19, 2016
@mwjames
Copy link
Contributor Author

mwjames commented Nov 19, 2016

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

Current output 123 • 456abc vs. expected output 123 • 456 • abc

@kghbln FYI

@mwjames mwjames merged commit d198c9e into master Nov 19, 2016
@mwjames mwjames deleted the templ-sep branch November 19, 2016 08:04
@kghbln
Copy link
Member

kghbln commented Nov 19, 2016

Just fetched the latest code on sandbox. Works perfect! Thanks!

@mwjames
Copy link
Contributor Author

mwjames commented Dec 10, 2016

By coincidence, @kghbln found:

image

with no sep= parameter therefore falling back to , as default.

{{#ask:
 [[Category:SMW releases]]
 [[Language code::en]]
 [[Is duplicate page::false]]
 [[Version::!Semantic MediaWiki 1.6.2]]
 | ?Version
 | ?Has release date#ISO
 | ?Has highlights
 | ?Is compatible with
 | format=template
 | sort=Version, Has release date
 | order=descending
 | limit=100
 | introtemplate=Version history intro
 | template=Version history
 | outrotemplate=Version history outro
 | intro=Here is the complete <b>version history</b>:
}}

@kghbln
Copy link
Member

kghbln commented Dec 10, 2016

By coincidence, @kghbln found:

Great, that we found this prior to the release.

@kghbln
Copy link
Member

kghbln commented Dec 10, 2016

@mwjames Should I open a separate issue allowing to track this?

@mwjames
Copy link
Contributor Author

mwjames commented Dec 10, 2016

Should I open a separate issue allowing to track this?

Maybe for the best. Also what I need is some kind of baseline that defines the behavior to avoid any future regression. We caught this just by mere chance!

{{#ask: [[List::2022]]
 |?Has page
 |?Has page
 |format=ul
}}

Expects to output Some, Some

Issue: The default sep is set to , which is a bit tricky because it cannot be decided whether a user made a deliberate decision or not. I will change the default to be empty and in case it is a list/ul/ol then the output will use , as default if no other sep was given. template by default will have no sep to avoid the case we just found.

@s7eph4n
Copy link
Contributor

s7eph4n commented May 10, 2017

Just for my understanding: This means that when I upgrade to 2.5 I have to modify all {{#ask}} calls with format=template and explicitly add a sep parameter? Or should it be valuesep?

@mwjames
Copy link
Contributor Author

mwjames commented May 10, 2017 via email

@s7eph4n
Copy link
Contributor

s7eph4n commented May 10, 2017

Ah, ok. Guess you had to break something to make it work properly. Just wish it wouldn't bite me now. :P
I'll change the {{#ask}} calls. That parameter is nice, but I will have forgotten all about it by tomorrow.

@kghbln
Copy link
Member

kghbln commented May 18, 2017

Needs more doculove

@stefahn
Copy link

stefahn commented Jan 26, 2018

I just came across this issue, since commas were missing on my wiki(s) since I updated SMW from 1.x to 2.5.5.

I use many ask queries and I didn't want to change every single one of them individually. Also it seems I didn't get the point of the valuesep thing. So I just added this line to my LocalSettings.php and all commas reappeared:
$smwgResultFormatsFeatures = SMW_RF_NONE;

However I find it strange that default output behaviour changes, when I update SWM.
Why is the default value of sep empty and not a comma?

@kghbln
Copy link
Member

kghbln commented Jan 26, 2018

Also it seems I didn't get the point of the valuesep thing.

The example should make it clear: https://sandbox.semantic-mediawiki.org/wiki/Template_sep_valuesep

However I find it strange that default output behaviour changes, when I update SMW.

Sometimes changes have to be made to bring more goodness. You always get occasional default setting changes, not just with SMW but e.g also with MW, but there is in most cases a way to retain the previous bahaviour to allow for migrating and you indeed found it: https://www.semantic-mediawiki.org/wiki/Help:$smwgResultFormatsFeatures

Perhaps in this case we should probably have added this to a breaking changes section in the RELEASE NOTES.

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.

None yet

4 participants