Commit graph

206 commits

Author SHA1 Message Date
jenkins-bot 4f8e06b97b Merge "Make it possible to disable the "cite_error" wrapper message" 2024-05-27 08:16:04 +00:00
C. Scott Ananian fc5f22b32e CiteParserHooksTest: make test compatible with removal of dynamic property
Whether the dynamic property is present or not, it should have a null
value when 'unset' -- and don't use `unset` to delete an *actual*
property when one is present!

Change-Id: Ifcb9492cc5c814d702c6e61e8231abfd8ea0647c
2024-04-12 15:23:14 -04:00
jenkins-bot 1fdc65548b Merge "Add strictly typed RefGroupItem class to Parsoid code" 2024-04-01 03:05:52 +00:00
jenkins-bot 51e569d888 Merge "Allow visualeditor-cite-tool-name-… messages to be missing" 2024-03-28 15:27:14 +00:00
thiemowmde 9e1c4645be Add strictly typed RefGroupItem class to Parsoid code
This does the exact same as the previously used generic stdClass
object, just strictly typed. Turned out to be surprisingly
straightforward, as proven by the small size of this patch.

I'm intentionally not adding anything new in this patch. For
example, the new class is perfect to write longer documentation
for every field. But this is for a later patch.

Change-Id: Ibf696f6b5ef1bfdbe846b571fb7e9ded96693351
2024-03-14 11:59:34 +01:00
thiemowmde 650d2c296d Add basic PHPUnit tests for Parsoid implementation
There is much more to test, but it's a start.

Intentionally build as pure unit tests to make them as fast as
possible.

Bug: T354215
Bug: T358652
Change-Id: Iae1a8086b8f2b9e5b11e0117bd3f19fdaa087df0
2024-03-12 09:06:16 +00:00
thiemowmde 2b32f15c8c Allow visualeditor-cite-tool-name-… messages to be missing
The information read from the …cite-tool-definition.json files is
effectively user input, even if only interface administrators can
edit it. Usually we carefully validate user input. But as of now
this code starts failing with all kinds of uncatched errors.
* An entry with no name, an empty name, or a name that's not a
  string will cause all kinds of undefined behavior.
* An entry with an empty title results in an invisible button.
* A missing message results in a technical <…> placeholder, even if
  the name is usually a sensible fallback.

Note that hard-coding titles as plain text strings in the ….json
file was already possible.

Change-Id: Iddcedbe859e86ac4c3f79a53d36237daff86c0db
2024-03-06 14:21:08 +01:00
jenkins-bot 46499cd9c0 Merge "Rename ReferencesFormatter to ReferenceListFormatter" 2024-02-07 18:00:04 +00:00
thiemowmde 7c75d44b8a Rename ReferencesFormatter to ReferenceListFormatter
I always found the name a little ambiguous. The fact that it outputs
an actual HTML list and not just some "references" – whatever that
means – is relevant, in my opinion.

Change-Id: I0d169455c8d2b42d62da4dccb8376c09fb6902bc
2024-02-07 18:20:02 +01:00
thiemowmde 802b495160 Make it possible to disable the "cite_error" wrapper message
… as well as "cite_warning". Both are extremely trivial and don't
really do anything by default. All they do is to add the prefix
"Cite error:" or "Cite warning:" to all error messages.

This patch will make it possible to disable both messages by
default, i.e. replace their default in en.json with "-" without
breaking anything. That's part of the plan outlined in T353695.
Local on-wiki overrides will continue to work.

Bug: T353695
Change-Id: I374800d0d0b837cd17ed3a1fdde20b70325b06de
2024-02-01 17:36:27 +01:00
thiemowmde 5eea8ebd90 Simplify ErrorReporter PHPUnit test setup
This was slightly overengineered ever since I4b1f890 and slowly became
more and more complicated over time, notably when withConsecutive was
replaced in Icb951b4. Turns out this was never really needed. It's
impossible to get more than one tracking category from this code path.

While we might add more tracking categories later that will most
probably not happen in this code path.

Change-Id: Ie32d17bac8d3518c985b18f83a846c3fb2bd053f
2024-02-01 17:28:44 +01:00
Adam Wight c36d1b90a5 Move extendsCount into parent ref item
Eliminates a complex shared structure in favor of more encapsulation.

Change-Id: I70efabf0ee263ac578472e16dc35047b0601b7ff
2024-01-10 11:55:16 +01:00
Adam Wight 0e01e39061 Encapsulate ref: groupRefs returned as objects
This patch only affects the consumers of groupRefs.

Bug: T353451
Change-Id: I1eff735dbc26dda07aa8ac7af9ea4ddc0906f5a4
2024-01-09 10:22:04 +01:00
Adam Wight f148c65078 Encapsulate ref: pushRef returns an object
This patch affects a few methods which use the output of pushRef.

Bug: T353451
Change-Id: I10b3fe89406c11cdaede92f18a4b96586ecaf5a0
2024-01-09 10:18:57 +01:00
Adam Wight 262fbe24eb Encapsulate ref object: limited to ReferenceStack
This encapsulation gives us field name, type validation and code
documentation.

This patch only affects ReferenceStack and continues to return
approximately the same array outputs to callers.  Some additional
information is included and the placeholder column has a new name.

Bug: T353451
Change-Id: I405fe7ac241f6991fd4c526bfbb58fbc34f2e147
2024-01-09 09:59:16 +01:00
Adam Wight 1434dc5ca6 Switch to a 1-based "count"
The previous patch deprecated the last conditional depending on magic
meanings of 0 and -1, so now we're free to let "count" take on a more
natural meaning: the number of times a footnote mark appears in
article text.

Includes a small hack to avoid changing parser output, by
artificially decrementing the count by one during rendering.  The
hack can be removed and test output updated in a separate patch.

Bug: T353227
Change-Id: I6f76c50357b274ff97321533e52f435798048268
2024-01-08 11:45:36 +01:00
Adam Wight 86edddc8c2 Use semantic field to test ref type
Stop relying on the magic number distinction between "count" = 0 and -1,
by explicitly testing the "name" field instead.

Bug: T353227
Change-Id: I9dce16b01814e19f508d45b927de570049f0e0f5
2024-01-06 16:46:56 +01:00
Adam Wight a6cb979d88 Inline constant for "placeholder" key
Minor refactoring of an internal field, which can be treated like the
other columns.

Change-Id: I255578694c5ab9f2ad3cbe232217af3cea60669c
2024-01-05 11:22:30 +00:00
Adam Wight fd648aec98 Store group in ref items
Encapsulate all information about a ref inside of the internal
structure, rather than relying on the container to be organized by
group.

Bug: T353451
Change-Id: I4c91e8089638b7655bf120402a4a5fcbd1b35452
2024-01-05 11:22:12 +00:00
Adam Wight ca6414320f More explicit test fixtures: key and count
These fields get automatic values during normal operation, but we
should make this explicit in tests which meddle with internals.  This
seems to add some clarity, and helps prepare for encapsulation.

Bug: T353451
Change-Id: I8b012a270f16139671f77ea04645d627b2fba87d
2024-01-05 11:21:58 +00:00
Adam Wight 76e6e870d4 Include more information in missing parent placeholder
This allows the subreferences to be collected together under a heading.

Bug: T353451
Change-Id: Ibf28f0baca14de8140c87b03ad4aa86d2f81a20d
2024-01-05 11:21:12 +00:00
Adam Wight 5a69c54900 Render list-defined parent without a backlink
In this case, there was never a ref with this name in the article so
no backlinks should be rendered.

TODO:
* test case with empty parent backlink and LDR parent

Bug: T353451
Change-Id: I8a7abd05a48ce83da3beb92b15e894d53252bd33
2024-01-05 12:07:22 +01:00
Adam Wight ddf5cb2458 Remove test for a private method
Testing internal methods is brittle.  This code path is already
covered by parser test "Valid follow="…" after it's parent"

Bug: T353451
Change-Id: I3b7a4b9962de1f25a7b57f82d80813219d633594
2024-01-04 17:07:36 +01:00
jenkins-bot ab20cb3cdf Merge "Rename appendText() to resolveFollow()" 2024-01-04 15:29:44 +00:00
Adam Wight 5d1335e279 Explicit parameter names for all test fixtures
This is much more readable.  Patch changes nothing.

Bug: T353451
Change-Id: I72b58881a7329dbe98659553b84e53896ccafc2b
2023-12-21 20:59:25 +01:00
Adam Wight e0e8b2dc33 Drop constant parameters
Some tests didn't vary the key parameter, so converting to a
constant.

Change-Id: I9a42352573cc16fec11a799b878c06751eb03fc8
2023-12-21 20:52:43 +01:00
thiemowmde d73a76dce6 Rename appendText() to resolveFollow()
There is only 1 user left after Icf16965.

Bug: T353266
Change-Id: I4cafdcbe0a23dd7950613a385cb552e7a84e7f26
2023-12-19 14:49:52 +01:00
jenkins-bot 9b87bc717d Merge "Various cleanups to PHPUnit test mock setup" 2023-12-18 12:36:11 +00:00
thiemowmde 742a9ffbf5 Track warnings separately in ReferenceStack
Check out how this gets rid of so many "to do" as well as
"deprecated" comments.

Next qustion: The elements in the stack become more and more
complicated. It's probably worth converting them from arrays into
first-class objects. But this is for another patch.

Bug: T353266
Change-Id: If14acd1070617ca8c4d15be6b1759bd47ead4926
2023-12-15 16:41:04 +01:00
thiemowmde 9304e24551 Various cleanups to PHPUnit test mock setup
For example, use convenient upstream methods, and generally make the
test setup a bit more readable.

Bug: T353227
Change-Id: Ifab71041fcc3f804315793ca7b783f84829c7a0f
2023-12-15 11:45:35 +00:00
thiemowmde 4377f0923d More simple and consistent @covers and @license tags
Same arguments as in Iafa2412. The one reason to use more detailled
per-method @covers annotations is to avoid "accidental coverage"
where code is marked as being covered by tests that don't assert
anything that would be meaningful for this code. This is especially a
problem with older, bigger classes with lots of side effects.

But all the new classes we introduced over the years are small, with
predictable, local effects.

That's also why we keep the more detailled @covers annotations for
the original Cite class.

Bug: T353227
Bug: T353269
Change-Id: I69850f4d740d8ad5a7c2368b9068dc91e47cc797
2023-12-15 12:12:16 +01:00
thiemowmde c794962df7 Use short fn() syntax in tests where it makes sense
We can use this syntax now. It was introduced in PHP 7.4.

Bug: T353269
Change-Id: I5404b33b654efb01171fa2b4ad3925170ffd0e56
2023-12-14 08:05:01 +00:00
thiemowmde 04208b5fd1 Remove PHPDocs that just repeat what the code already says
We removed a bunch of now redundant docs already, see e.g. Ie0692fa.

Change-Id: I55c62d935bdec8bedaebc2666fca3eb17309b0c7
2023-12-13 12:44:41 +01:00
thiemowmde fee8606db6 Avoid a few isset() in favor of more recent syntax
As well as replacing a few `=== null` comparisons with the new ??=
operator.

Bug: T353227
Change-Id: I5b273f452d1e46d37fc28861b54c4e1f19a7a65a
2023-12-12 12:13:42 +00:00
jenkins-bot f455dd3f79 Merge "Re-arrange code in preparation for T298278" 2023-12-05 14:53:42 +00:00
thiemowmde 5f5e9ec9f0 Re-arrange code in preparation for T298278
This patch makes only sense together with I5a64ac4 where it is split
from. See I5a64ac4 for details.

The idea is that this patch just re-arranges the code without making
any changes to how the code behaves. This leaves a minimal change
behind that's much easier to revert, if needed.

Bug: T298278
Change-Id: Ie78313b7f3ac1ec7bce5ac7512e60a3bb011480a
2023-12-04 08:29:53 +01:00
gerritbot 3d34307f87 Update StaticUserOptionsLookup's FQN
User-options related classes are being moved to
the MediaWiki\User\Options namespace in MediaWiki Core;
reflect that change here.

Bug: T352284
Depends-On: I42653491c19dde5de99e0661770e2c81df5d7e84
Change-Id: I22ff2effcf9b7f2162f5d57608d8ec3651b48dd7
2023-11-29 17:55:07 +00:00
Daimona Eaytoy c71109b97b Update tests for PHPUnit 9.6
- Avoid withConsecutive()

Bug: T342110
Change-Id: Icb951b461c5e6ea1ced2ab8a183c53265d4a2b4f
2023-11-22 00:15:20 +01:00
gerritbot 198ae5b21c Replace some moved Title class uses, now MediaWiki\Title\Title
Bug: T321681
Change-Id: Idde5511e075fcec40cad12c7369abec3ee4d096c
2023-08-19 04:13:55 +00:00
jenkins-bot 0fc08a2ac7 Merge "Replace extremely slow parser test with fast unit tests" 2023-07-28 01:05:39 +00:00
thiemowmde 5aa6cb0c7b Replace extremely slow parser test with fast unit tests
This parser test is a bit obscure, in my opinion. We added it in
I8c4de96 to make sure we don't get thousand separators in most
places.

We continued reworking the code since then. By now it's effectively
impossible to "accidentally" get thousand separators. The
problematic methods from the Language class are not even accessible
any more from this code.

To make the tests more robust we now use createNoOpMock (done via
the previous patch) where it matters, specifically for all Language
and Parser mocks. This proves the problematic Language methods are
never called.

Bug: T253743
Bug: T238187
Change-Id: I9bfe1f4decfaf699996da63e19473c2c0d581d9d
2023-07-28 00:32:38 +00:00
jenkins-bot ebd355c067 Merge "Replace all Language and Parser mocks with no-op mocks" 2023-07-27 18:59:53 +00:00
thiemowmde 2aa421a021 Replace all Language and Parser mocks with no-op mocks
Both Language and Parser are extremely complex classes with hundreds
of public methods. We really want to make sure we are not depending
on anything unexpected from these classes. If calls are made into
these classes we want to know exactly what is called.

Doing this also showed that some mocked methods are not even needed.

Change-Id: Icdfff6c07be78a47bf7cadb1813a72581a51272a
2023-07-27 10:00:28 +00:00
thiemowmde e750cc2ed2 Remove unused "HTML message" cite_references_no_link
I believe there is no reason to keep this. The only reason might be
a known wiki that uses this as a feature. But such a wiki doesn't
exist:
https://global-search.toolforge.org/?q=.&regex=1&namespaces=8&title=Cite.*no.*link

Bug: T321217
Change-Id: I128d4383da48f9bdda92f03b212ef3997b3a4802
2023-07-27 11:55:13 +02:00
thiemowmde 08814c8e38 Remove (revert) expensive parsing of 1-character message
This reverts a very tiny part of Ib3fdc89 from 2 weeks ago. The
reasons are explained in Ib3fdc89. Short version:
* The ->parse() calls have drastic performance implications.
* Allowing wikitext and HTML in this message also makes T321217
  worse.

The new message "cite_reference_backlink_symbol" is kept and still
used in the UI. Just not in these two messages any more. This is a
minor redundancy we want to get rid of at some point. But it's not
critical for the moment. This will be done as part of T321217.

Nothing will break on the wikis. Some wikis have customizations for
"cite_references_link_one" and "cite_references_link_many" in place.
This will continue to work as before Ib3fdc89.

Bug: T339973
Change-Id: I933771e3ad67cd530bcf5ee8469cef35ea1070d2
2023-07-21 14:05:31 +02:00
Jon Harald Søby c66371b3d9 Move Cite-specific settings from WikiEditor
The WikiEditor extension has a button and some help text that
is only applicable if the Cite extension is enabled. Move
that (with some modifications) to the Cite extension instead.

Bug: T339973
Depends-On: I8256660f9c6886d6764b45735284e00308fc56e5
Change-Id: Ib3fdc897dd3330f69c5832003d4c3cb1e6dba2f3
2023-06-28 20:22:14 +02:00
thiemowmde 269f726cff Remove inline @var type hints that are not needed
This is mostly because recent IDEs can understand createMock() quite
good. We usually don't add such hints every time we use createMock().
We would have a million of them. ;-)

Change-Id: If9e37807a6945c4408d374fc97664cd636020ffd
2023-06-05 16:37:03 +02:00
Umherirrender 66159e5b78 tests: Make PHPUnit data providers static
Initally used a new sniff with autofix (T333745)

Bug: T332865
Change-Id: Ib86d0fb2d3ea734db46b266faede7b4588fae075
2023-05-20 12:03:41 +02:00
Tim Starling 5315297f38 Migrate CiteVisualEditorModule to a virtual file callback
Depends-On: I97d61b5793159cea365740e0563f7b733e0f16de
Bug: T47514
Change-Id: Iabfbb6751707813b7ec68f49b35441ab5dbb5622
2023-05-05 16:25:14 +10:00
jenkins-bot d8aee328cf Merge "Replace string|false in CiteParserTagHooks with nullable ?string" 2023-03-20 09:36:39 +00:00