Commit graph

27 commits

Author SHA1 Message Date
Thiemo Kreuz 0fe9dbb366 Don't expect objects by reference in hook handlers
The motivation for this patch is to make the code less complex, better
readable, and less brittle.

Example:

public function onExampleHook( Parser &$parser, array &$result ) {
    /* This is the hook handler */
}

In this example the $result array is meant to be manipulated by the
hook handler. Changes should become visible to the caller. Since PHP
passes arrays by value, the & is needed to make this possible.

But the & is misplaced in pretty much all cases where the parameter is
an object. The only reason we still see these & in many hook handlers
is historical: PHP 4 passed objects by value, which potentially caused
expensive cloning. This was prevented with the &.

Since PHP 5 objects are passed by reference. However, this did not
made the & entirely meaningless. Keeping the & means callees are
allowed to replace passed objects with new ones. The & makes it look
like a function might intentionally replace a passed object, which is
unintended and actually scary in cases like the Parser. Luckily all
Hooks::run I have seen so far ignore unintended out-values. So even if
a hook handler tries to do something bad like replacing the Parser
with a different one, this would not have an effect.

Removing the & does not remove the possibility to manipulate the
object. Changes done to public properties are still visible to the
caller.

Unfortunately these & cannot be removed from the callers as long as
there is a single callee expecting a reference. This patch reduces the
number of such problematic callees.

Change-Id: Ib3a9da257b50326d569ab1973b523c952963c16b
2018-05-17 17:09:55 +00:00
Thiemo Kreuz 8a42f61697 Remove all default "return true" from all hook handlers
This is the default for many years now. Returning true does nothing. It's
identical to returning nothing (null). The only meaningful value a hook
handler can return is false, and even this is meaningful only for very
few hooks.

TL;DR: A "return true" in a hook handler is always meaningless, dead code.

I'm interested in this because we (WMDE) might start working on this
extension soon and I want the code to be small and easy to maintain.

Change-Id: If4f32a55cdc38a3cc8af286d1cca7c0089bbfc43
2018-05-15 10:43:23 +02:00
Ed Sanders b8a9115d27 Use resource loader for icons
Change-Id: I6614d2b33936e4e4f04412b76df67dc5554bf3f7
2018-04-11 17:56:19 +01:00
Thalia a2722ca947 Add separate message for missing reference in references list
Message indicates that a preview of the reference is missing,
instead of implying that the user was trying to edit it.

Bug: T188682
Change-Id: I5f5f8d5d0910ab2608696bbed380d4592cb6c7f1
2018-03-24 17:32:55 +00:00
Thiemo Mättig bbc1f2c91d Use standard form for @license tags
See https://spdx.org/licenses/

Change-Id: Ic091ebc3844abcd6de90b3241382fb4732200a6d
2018-03-20 03:18:37 +00:00
James D. Forrester 43344b56b0 VisualEditor: Describe reflist responsiveness changes too
Bug: T160589
Change-Id: Idb590bf0f3edbe8d056f544a148323da30211467
2018-02-22 16:01:46 -08:00
Ed Sanders 8e5ae11be0 Create first unit test for reference diffs
Depends-On: Ia34b535c34aeed933164a18a0be76d7c0ac28bc6
Depends-On: Ic70bd3e8ba090c2d174164fd1ecebdf6f2225254
Change-Id: Ib8b485663dea6fcc50c0197451d0a553e5d89c27
2018-02-13 19:22:58 +00:00
Ed Sanders 495d9420f0 Treat template-generated reference lists as real ones
This will allow them to auto-update, and be used in the
visual diff.

We record a templateGenerated attribute to prevent them
being edited, and to ensure they roundtrip back to
templates.

Bug: T52769
Change-Id: I4460d2c98166581e942e35921b20091990f5f6c7
2018-02-07 10:48:57 +00:00
James D. Forrester 0999d771b6 doc: Bump copyright year
Also make the overly-terse statement clearer.

Change-Id: I752bb82444bf7cb41480076bace42331dc35a598
2018-01-02 17:05:50 -08:00
Thiemo Mättig 9c8ed18938 Remove some obscure comments
A good bunch of these comments literally repeats what the code already
says.

Change-Id: I9c128f748971bf20a61a85ed57d3261d27c465f0
2017-12-29 12:21:53 -08:00
Phantom42 67ed343ecc Add phan configuration for static analysis
Bug: T179554
Change-Id: I2bfd52c08aac1aa8f34e0664e6314835f79a0324
2017-12-29 11:50:01 -08:00
Umherirrender 0802a73cae Improve some parameter docs
Change-Id: I7d50eaabfba309a62d1dbd16ded0cbcab3beb82f
2017-10-06 21:22:30 +02:00
Ed Sanders a3a4bd945b VE: Support 'responsive' attribute
Bug: T53260
Change-Id: I1e0ae39e8c30653b7ba0f537723a4bcd79ac3162
2017-09-19 17:03:39 +01:00
Arlo Breault d465b4e001 API: Expose $wgCiteResponsiveReferences via meta=siteinfo
Bug: T159894
Change-Id: I2404999ab11b5cf7b740ae43696c4676ab1b6d22
2017-09-11 23:04:25 -04:00
Thalia ed8b563add VisualDiff: Show less information about ref nodes
If a ref node is highlighted as changed because its index
has changed (e.g. because an earlier reference was inserted
or removed), describe this more elegantly.

Bug: T170235
Bug: T171377
Change-Id: I2513bb82099a92529516e4e217e61a2d0a2dd43b
2017-09-05 14:01:39 +01:00
jenkins-bot 8b3ca486a6 Merge "Visual diff: Show correct reference indices in diff" 2017-06-26 19:35:32 +00:00
Thalia 1a5d90b4e3 Visual diff: Show correct reference indices in diff
Bug: T162819
Change-Id: I7eec4ee12a24ff99388cc0c95a24d50f321511f9
2017-06-26 19:19:03 +00:00
jenkins-bot 66dabf8821 Merge "VisualEditor: Add a placeholder to MWReferenceDialog to make it less awful when empty" 2017-06-20 19:19:03 +00:00
Kunal Mehta 635ee1cce0 build: Updating mediawiki/mediawiki-codesniffer to 0.9.0
The following sniffs are failing and were disabled:
* MediaWiki.Commenting.FunctionComment.MissingParamComment
* MediaWiki.Commenting.FunctionComment.MissingParamName
* MediaWiki.Commenting.FunctionComment.MissingParamTag
* MediaWiki.Commenting.FunctionComment.MissingReturn
* MediaWiki.Commenting.FunctionComment.ParamNameNoMatch
* MediaWiki.Commenting.FunctionComment.WrongStyle
* MediaWiki.FunctionComment.Missing.Public
* MediaWiki.WhiteSpace.SpaceBeforeSingleLineComment.NewLineComment

Change-Id: I5a7f01d6d5d8b56583940524b251d4de1ca09f5e
2017-06-19 19:32:30 -07:00
James D. Forrester 347f21d11e VisualEditor: Add a placeholder to MWReferenceDialog to make it less awful when empty
Change-Id: I086d7243127ee411ea043bf29f4d4a5ae5d657a0
2017-06-17 01:21:48 +00:00
Bartosz Dziewoński f07c7a2e10 Add appropriate OOjs UI icon pack dependencies
modules/ve-cite/ve.ui.MWReferenceDialog.js
* Add 'alerts' as it uses 'alert'
* Add 'interactions' as it uses 'settings'

Bug: T166730
Change-Id: I39d98c197220d463f18d473a9605e242ec9755a9
2017-06-03 12:45:02 +02:00
James D. Forrester 9a17d9342b Describe group changes for references and references lists
Bug: T160589
Change-Id: Ifccafdf08704f67027dae2703ff2ded809fb6ab7
2017-05-20 14:26:58 +00:00
James D. Forrester 0d20873fa6 doc: Bump copyright year notice, 'team' name
Change-Id: Ia090c417a1aa716b255613199b5e49616bf0517a
2017-04-20 16:50:43 +00:00
Ed Sanders 5b477ce3a1 Bring in wikitext paste tests from ve-mw
Depends-On: Ibea6994a208e4b0b1022896eb31dd2f36f0fd6c6
Change-Id: I244570c39e2f3595137c02e62cb91ba8eeab8a4a
2017-04-20 16:27:50 +00:00
Antoine Musso 5fcecf9e5f Skip registering RL modules depending on VisualEditor
When VisualEditor is not installed, there is no point in registered
resource loader modules that depends on it.  A use case is trying to run
tests for the MediaWiki tarball. It comes with Cite but without
VisualEditor.

The patch is based on GuidedTour patch by Matthew Flaschen
https://gerrit.wikimedia.org/r/#/c/305691/ for T143297

Change-Id: Idf769e0149f93c099a94b1b7a6cb203273dab881
2016-10-14 23:45:03 +02:00
Moriel Schottlender fdb25a79aa Followup Ic6afacbf2a: Fix test file paths
The current path for the test files was wrong and broke qunit test
running in debug mode. This fix corrects the path for both debug
and non debug mode.

Change-Id: I0e2f3296daff1aff5d417a9e8f8ae7eadfe78982
2016-09-20 13:55:42 -07:00
addshore c1de73eb32 Move php files in includes directory
Change-Id: Ic6afacbf2a944954826c9d68dd292c28f1c731da
2016-09-20 15:33:10 +01:00
Renamed from CiteHooks.php (Browse further)