Note how this code was broken since 2018 (Iff480bc). In this execution
path, $val is a string. There is no $val['dir'].
Luckily, this was dead code since 2008 already. See
https://phabricator.wikimedia.org/rECIT448a99da5108c26ce88d3df7cf5df2b5b5b1d1d3
line 283 on the right.
Bug: T237241
Change-Id: I671f3379a124a2644a9b0eac38d46c59106980a7
The main motivation here is to cover the fallback code that was moved
in I20c814d. At some point we might touch this code again.
Bug: T238194
Change-Id: I0ab8a34b09790f42b10376eb3730c3b3c4ef53d2
This code is typically not executed on special pages and such, so we
assume the isArticle check doesn't make much of a difference. The main
difference we are aware of is that this will exclude previews.
Bug: T214493
Change-Id: I5155329b8a549adbd3b17c1f1014bb8bbc6768f4
1. Most existing CiteTests can be unit tests. They run so much faster
this way.
2. I modified some test cases to cover all trim() in the code.
3. The strict type hint in CiteHooks is removed because the parameter
is not used. Having a hard type hint for what is effectively dead code
makes the code more brittle for changes done outside of this codebase.
Change-Id: I1bff1d6e02d9ef17d5e6b66aeec3ee42bba99cf4
This fixes a series of issues:
* There is nothing about a "frame" in the Cite class any more.
* There is no addModules() call in the Cite class any more.
Change-Id: I20c814d46c26825c5c07eab0a5586de3a531eee7
To be honest I don't get why this lazy registration was done in the
first place. None of the 4 other hooks should ever be called before
the ParserFirstCallInit hook got called.
Also, under which circumstances can the ParserFirstCallInit hook be
called more than once?
Both scenarios would be wrong, as far as I'm concerned. Either I'm
missing something, or this code can indeed be simplified. Maybe it was
something to make it more compatible with older MediaWiki versions?
The only reason I can think of is: in all situations that do not
involve a parser, having the 4 extra hooks registered is pointless.
Does this waste space and/or runtime in the $wgHooks registry?
Change-Id: I5ef1495f4ce7bce940fa5f8e700af3d2c4851a01
Two motivations:
1. I want the two deeper nested functions guardedRef() and
guardedReferences() to have less side effects.
2. In guardedReferences() guardedRef() is called. Both set the
property. That's redundant. The new code avoids this.
Change-Id: I48146f8b6d91122a904be0a552ffe3b03bc0481f
Main motivation is to make the code easier to test, and easier to
extract to smaller services.
Does this make sense? I'm not sure any more. One can argue that
everything Cite does happens in the context of a specific Parser. Why
shouldn't the code have access to this Parser?
Change-Id: I9d0cb44d96ec70a56af57f86aeb1f264f52c8bc4
Can use a shortcut where we pass the expected value directly. Verified
that we're still asserting equality.
Change-Id: I63512488c50e599df23d5dae2a5064218e311e90
Note it doesn't make a difference if this is behind the feature flag or
not. It should always be forbidden, and in fact is: Either the follows
attribute is unknown, or the combination is forbidden.
Bug: T236256
Change-Id: Iebbb2d1d5bab183ab0590b8a7a7f6e79d319b72c
What we find critical is:
* That all tests relevant for book referencing are in a separate file.
* That unimplemented stuff is marked with TODOs.
Not having to move tests to another file allows for nice diffs.
I tried to order the tests as good as I could. E.g. have all tests with
a group="…" next to each other, followed by all with a follow="…".
Change-Id: Idc1d9e7843b341235ab3d8ebe398e01946eb1845
Any time the book referencing attribute is used in a page,
permanently tag that page with the `ref-extends` property, so
that it can be watched and cleaned up if necessary.
Bug: T237531
Change-Id: Ice5d9d8f7a305702cdc7c2a55d4147c4f79b5881
Incremental patch which extracts the refines attribute from the tag.
Doing this now to allow the calling function to have responsibility
for doing something with the attribute value.
Bug: T237531
Change-Id: I59bb409bedd8e6ed06268e705e02e8ffb45b1f0e
Encapsulate the feature tests in dedicated files. These are picked
up by the test runner for matching glob `tests/parser/*.txt`, as can
be shown by,
phpunit.php --testsuite parsertests --filter=bookRef
Also adds TODO comments to some tests, documenting how the current output
will not match the fully implemented code's results.
Bug: T236256
Change-Id: Ie3e769c84856256180754aeff417da893a84b479
These tests document the current status quo, and are meant to change
with every patch that makes the code for refined references more
feature complete.
Bug: T236256
Change-Id: I8c11b1decc36b86e7f7d1919cc39d0c16a200055
Note this is intentionally testing a private method. As of now, the
code is so heavily entangled, it's not yet possible to test individual
aspects without calling private methods. The plan is to slowly increase
the overall test coverage, and the start restructuring the code as
necessary.
Change-Id: Ib3b01bddaffd0469fb66979c67c8114a5807df6d
I was particularly suprised by the conditions that checked if
`$parser->extCite !== $this`. This can never happen. Maybe it was
possible in a very old version of this code, but it is not any more.
Change-Id: I049ff4109a747eb9dbf325c24cf20f65753827dd
As of now, this patch does not touch the existing code. However, the
goal is to remove a lot of the related code from the Cite class. This
will be done in later patches. This here is a separate patch to make
reviewing the later patches much easier.
The existing parser tests should be proof enough this chain of patches
is not changing any behavior.
Change-Id: I27ae972f81071bb4036bd452560768fae409417b
