Page MenuHomePhabricator

Add support for removing sitelinks
AcceptedPublic

Authored by Lucas_Werkmeister_WMDE on Feb 9 2018, 4:11 PM.

Details

Reviewers
Magnus
Patch without arc
git checkout -b D969 && curl -L https://phabricator.wikimedia.org/D969?download=true | git apply
Summary

Setting a sitelink to the empty string removes it, so this was already possible, but not via the “-” syntax.

Diff Detail

Repository
R2010 tool-quickstatements
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

Lucas_Werkmeister_WMDE requested review of this revision.Feb 9 2018, 4:11 PM
Lucas_Werkmeister_WMDE created this revision.
WARNING: I haven’t tested this change, since setting up a local QuickStatements installations seems to be close to impossible (I had enough problems just with the UI part when I was working on the CSV format, and this is backend stuff).
public_html/quickstatements.php
805

Before discarding the value, we could, if it’s nonempty, first compare it with $i->getSitelink ( $command->site ) (cf. commandSetSitelink a bit further up), and reject the action if it doesn’t match. This would be a kind of “only remove this sitelink if it matches what I expect, otherwise leave it alone”. Do you think that would be useful?

public_html/quickstatements.php
805

Or, phrased differently: “-Q1234|xywiki|” would be “remove the xywiki sitelink”, whereas “-Q1234|xywiki|foobar” would be “remove exactly this sitelink, do nothing if it’s not there”.

Magnus accepted this revision.May 2 2018, 12:23 PM

I have checked in a manually edited version (got "error: patch failed: public_html/quickstatements.php:801; error: public_html/quickstatements.php: patch does not apply") to master.

Checking $i->getSitelink($command->site) might be useful, but I think we can do without that for now.

This revision is now accepted and ready to land.May 2 2018, 12:23 PM