View source for Module talk:Commons link

==Code review?==
{{ping|RexxS}} Remember [[User talk:Mike Peel/Archive 51#Expanding the use of wikidata in Commons interwiki link templates?|the discussion we had]] on {{U|Mike Peel}}'s talk page? I've implement Lua code to perform cross-checking against multiple Wikidata properties to look for Commons galleries or categories (or both). That is this module, which is currently in use at {{tl|commons and category}} and {{tl|commons and category-inline}}. I'd like to use it for {{tl|commons-inline}}, and perhaps {{tl|commons category}}. Before I do anything, would you be willing to look at the code and give me feedback? As usual, the testcases are [[Module:Commons link/testcases|here]]. Thanks for any help or suggestions you can provide! — [[User:Hike395|hike395]] ([[User talk:Hike395|talk]]) 15:48, 15 March 2020 (UTC)
:
: {{re|Hike395}} I've just checked the results at [[Module talk:Commons link/testcases]] and they look reasonably comprehensive, and absolutely accurate. I've also now gone through the code at [[Module:Commons link]] and it looks good{{snd}} a lot of the code is similar to functions that I've used in the past, so I'm confident that your algorithms are correct. The only problems I foresee are when editors manage to misuse the code, and you need to decide whether it is better to let a function fail with a sensible error message or catch the error and let the function continue with a null result. For example:
:* <code><nowiki>{{#invoke:Commons link |getGallery |qid=Q0 |search=}}</nowiki></code> → {{#invoke:Commons link |getGallery |qid=Q0 |search=}} ''(← bad entity-id)''
:* <code><nowiki>{{#invoke:Commons link |getGallery |qid=Q68979196 |search=}}</nowiki></code> → {{#invoke:Commons link |getGallery |qid=Q68979196 |search=}} ''(← entity-id exists, but has no links)''
: You'll usually have to work out in your own mind whether it's possible to abuse the local functions, such as at line 15 <code>qid = mw.wikibase.getEntityIdForCurrentPage()</code>{{snd}} is it possible for the code to be running on a page that has no linked EntityID? If so, what happens when qid is returned as nil on line 16? Similarly for line 20, if <code>mw.wikibase.sitelink(qid)</code> somehow manages to return nil (no sitelink). Incidentally, I'd recommend using the latest calls: <code>mw.wikibase.getSitelink</code> is current, while <code>mw.wikibase.sitelink</code> is a legacy alias and shouldn't be used in new code (per [[:mw:Extension:Wikibase Client/Lua #Legacy aliases]]).
: Anyway, I'd have no qualms about introducing the code into production, as long as you're ready to catch any feedback from editors who spot errors. To paraphrase [[Helmuth von Moltke the Elder|von Moltke]], "No code survives contact with the user." Cheers --[[User:RexxS|RexxS]] ([[User talk:RexxS|talk]]) 16:51, 15 March 2020 (UTC)
::{{ping|RexxS}} Thanks for the code review! Most of the code has nil checks (because I copied a lot of code fragments from your code in [[Module:WikidataIB]]). I've cleaned up the remaining problems that I could find, and added tests to cover the problems you found. Next, I'll propose use at [[Template talk:Commons-inline]]. Thanks again! — [[User:Hike395|hike395]] ([[User talk:Hike395|talk]]) 00:40, 16 March 2020 (UTC)
: {{ping|Hike395}} I don't know enough Lua to review the code, but I'd like to suggest a different way to go about this. In an ideal world, I think we would want the following behaviour:
:# If there is a gallery link, show that.
:# If there is a category link, show that (in addition to the gallery link if both are available).
:# If there are additional category links, such as through {{WDP|P7561}}, then also show those.
:# If not, show the search link.
:# With tracking categories to say which options are being used, and extra tracking categories if they are locally defined.
: I think that means merging {{tl|Commons}}, {{tl|Commons category}} and {{tl|Commons and category}}, which would make things simpler here if there is a good Lua back-end to run them. Personally, I'd prefer we drop support for {{WDP|P373}} and {{WDP|P935}} as early as we can, in favour of the sitelinks, but keeping fall-back options for now seems OK. Thanks. [[User:Mike Peel|Mike Peel]] ([[User talk:Mike Peel|talk]]) 21:34, 17 March 2020 (UTC)
:: {{ping|Mike Peel|RexxS}} The functions in the module can certainly be used to create what you suggest. I very much like the combination of (1), (2) and (4). It's pretty close to what I want to do at {{tl|Commons-inline}}, except I was planning on showing ''either'' a gallery ''or'' a category link, and you're proposing showing both if they both exist. Probably the easiest way to proceed is to change the behavior {{tl|Commons and category}} and {{tl|Commons and category-inline}} to reflect your suggestion.
:: Note that in order to do both (1) and (2), you'll probably need to keep both {{WDP|P373}} and {{WDP|P935}}, because you need those to complement the Commons sitelink, which can only store the gallery or the category, but not both. — [[User:Hike395|hike395]] ([[User talk:Hike395|talk]]) 01:11, 18 March 2020 (UTC)
:::{{ping|Mike Peel|RexxS}} For fun, I implemented part of Mike's idea [(1), (2), and (4)]. It was quite easy: I put it into getGalleryAndCategory() in the sandbox module, and updated {{tl|Commons and category/sandbox}} to use it. If we decide to adopt this, it will change the behavior of {{tl|Commons and category}} if there is exactly one of a gallery and a category -- instead of searching for the other one, it will just present one. If there are neither, it will offer a search for both. 
:::For extra credit, I wrote this in such a way that {{tl|Commons}} and {{tl|Commons category}} can become wrappers around {{tl|Commons and category}}. {{tl|Commons}} behavior would change (a category link might show up). {{tl|Commons category}} behavior would change (a gallery link might show up). But it should work. 
:::WDYT? — [[User:Hike395|hike395]] ([[User talk:Hike395|talk]]) 04:20, 18 March 2020 (UTC)
:::: {{ping|Hike395}} I like this, hopefully the wider community will too. With the sitelinks to both gallery and category, the standard setup on wikidata is like that at {{WDQ|Q2322184}} (a random example, the last one I was editing yesterday). There you can get the gallery sitelink from {{WDQ|Q2322184}}, and follow {{WDP|P910}} through to {{WDQ|Q87840009}} to get the category sitelink. You'd need to include a namespace check on that, as in categories you'd have to do the reverse via {{WDP|P301}}. No need to keep {{WDP|P373}} and {{WDP|P935}} in that case (others I'm still working through to clean them up though!). Thanks. [[User:Mike Peel|Mike Peel]] ([[User talk:Mike Peel|talk]]) 09:16, 18 March 2020 (UTC)

== Lua error ==

Some articles are showing "Lua error: bad argument #1 to 'getBestStatements' (string expected, got nil)". Examples: [[Pastry]], [[Remote control]]. That is because <code>qid</code> is nil in <code>mw.wikibase.getBestStatements(qid, "P373")</code> at line 184. I'm hoping {{u|Hike395}} will investigate. [[User:Johnuniq|Johnuniq]] ([[User talk:Johnuniq|talk]]) 10:28, 2 August 2021 (UTC)

:I know what it is. Preparing a fix now. — [[User:Hike395|hike395]] ([[User talk:Hike395|talk]]) 12:56, 2 August 2021 (UTC)

::{{Fixed}} --- I added a test case to check [[Pastry]]. — [[User:Hike395|hike395]] ([[User talk:Hike395|talk]]) 13:11, 2 August 2021 (UTC)

== Edit request 8 September 2024 ==

{{edit template-protected|answered=yes}}
Hi, could you please apply [{{fullurl:Module:Commons_link/sandbox|diff=prev&oldid=1244660935}} this change] to the main module?

[[User:Od1n|Od1n]] ([[User talk:Od1n|talk]]) 11:55, 8 September 2024 (UTC)
:{{done}}<!-- Template:ETp --> &mdash;&nbsp;Martin <small>([[User:MSGJ|MSGJ]]&nbsp;·&nbsp;[[User talk:MSGJ|talk]])</small> 20:05, 9 September 2024 (UTC)