Commit graph

20 commits

Author SHA1 Message Date
Thiemo Kreuz e5cda65fbe Remove single use classes from the use section
The name of the base class in tests is guaranteed to only occur a
single time in a file. There is not much value in making it relative,
and requiring it to appear in the use section. Especially because it
is in the root namespace.

This reflects what I once encoded in the sniff
https://github.com/wmde/WikibaseCodeSniffer/blob/master/Wikibase/Sniffs/Namespaces/FullQualifiedClassNameSniff.php
I wish we could pick this rule and use it in our codebases. But it
seems it is to specific and can't be applied on all codebases, hence
it can't become part of the upstream MediaWiki rule set. At least not
at the moment.

Change-Id: I77c2490c565b7a468c5c944301fc684d20206ec4
2019-12-17 14:57:55 +01:00
Thiemo Kreuz 51ff3cc819 Several code cleanups after getting rid of cloning
* Use the Html class to safely create HTML code.
* $this->referenceStack can not be null any more.
* $this->inReferencesGroup is not needed during output, only when
  parsing tags.
* Replace ReferencesStack::getGroupRefs() as well as deleteGroup()
  with a combined popGroup() that does both things.
* Extract the code responsible for the "responsive" behavior to a
  separate function.
* Some TestingAccessWrapper are not needed.

Change-Id: Ie1cf2533d7417ae2f6647664ff1145e37b814a39
2019-12-16 15:47:23 +01:00
Adam Wight ab07a3253c Remove Parser state from CiteErrorReporter
Finishes breaking the circular reference between Cite and Parser.

This patch also demonstrates how evil it is to allow the error reporter
to be called from anywhere, and have side-effects.  At least it's explicit
now.

Also fixes a bug where the inner error message would not be in the
interface language.

Bug: T240431
Change-Id: Ic3325cafb503e78295d72231ac6da5c121402def
2019-12-12 11:15:07 +01:00
Adam Wight a227395e3a Lazy instantiation of Cite
Only create a Cite object if we need one.  Never clearState, just
destroy and recreate later.

This makes it less likely that we leak state between parsers, and
saves memory and processing on pages without references.

It's also preparation to decouple Cite logic from state.

Change-Id: I3db517591f4131c23151c76c223af7419cc00ae9
2019-12-12 11:12:17 +01:00
Thiemo Kreuz 3f2aeb7e31 Rename two Cite… classes and clean up test setups
* All classes are in a Cite\ namespace now. No need to repeat the word
"Cite" all over the place.

* The "key formatter" is more an ID or anchor formatter. The strings it
returns are all used in id="…" attributes, as well as in href="#…" links
to jump to these IDs.

* This patch also removes quite a bunch of callbacks from tests that
don't need to be callbacks.

* I'm also replacing all json_encode().

* To make the test code more readable, I shorten a bunch of variable
names to e.g. $msg. The fact they are mocks is still relevant, and still
visible because these variable names are only used in very short scopes.

Change-Id: I2bd7c731efd815bcdc5d33bccb0c8e280d55bd06
2019-12-12 08:48:02 +01:00
Adam Wight f51060eaf4 Fix footnote mark after extends numbering glitch
The visible numbering needed to be rolled back after an extends.

Bug: T237241
Change-Id: I95404515110df1fa7e3279ea499577df0ed45ddf
2019-12-09 12:06:59 +01:00
Adam Wight 96db7944eb Cover rollback with tests
Fixed an unsafe array access during rollback.

Change-Id: Id9ee8976e3bae24501c18abf462e3e19894caff0
2019-12-03 09:53:23 +01:00
Adam Wight 4cf8faea48 Cover edge cases with unit tests
Change-Id: If715788292631f2e1f4cf970c1ae7c7fd7d514e1
2019-12-03 09:53:23 +01:00
Adam Wight 008526b3aa Can use extends before its parent
If `extends` is encountered before the parent ref, we reserve the
sequence number and leave a placeholder to record the link between
ref name and number.  This is necessary to render a list like,
"[1] [2.1] [2]", or to use subreferencing when the parent ref is
declared in the references tag.

When a placeholder is encountered during references section rendering,
it means that the parent was never declared.

Change-Id: I611cd1d73f775908926a803fae90d039ce122ab6
2019-12-02 17:14:11 +01:00
Adam Wight e9958d569b Formatter takes responsibility for rendering footnote mark
Pass the full ref structure from ReferenceStack to FootnoteMarkFormatter,
to give it control over the final rendering.  This is aligned with how
the FootnoteBodyFormatter directly scans over groupRefs.

Change-Id: I3294fd9366f01daa4250a5d481f4adbae84c72b1
2019-12-02 10:17:24 +01:00
Adam Wight 3f276388bf Split ref.number field
This was carrying the entire footnote marker, but subreferences need
to extract just the first (group ref sequence) part.  Storing number
and extendsIndex in two separate fields gives us more flexibility
during rendering, for example these might use two different symbol sets.

Change-Id: I75bd6644c336036f9e84ba91e1c35e05bc1ca7f3
2019-12-02 10:17:24 +01:00
Adam Wight 5dfe633b33 Include name in ref structure
This will become useful in I611cd1d7, when we calculate ref link text
in FootnoteMarkFormatter.

Change-Id: I729701614829ccbca4c243c181ded13f354d1103
2019-12-02 10:17:24 +01:00
Adam Wight 0c908ced4c Fix impossible tests
Validation blocks (name==null && text==null), so it should not be a
test case.  Give the text a non-null value.

Also adds a check for missing test data.

Change-Id: I0f02206e2221805f5a2f8eaa163ed237cfb8d777
2019-12-02 10:15:29 +01:00
Thiemo Kreuz 13598ba11e Render nested references
Forked from Icd933fc983.

Bugs and unimplemented features are documented as TODOs in the parser test
fixtures.

Bug: T237241
Change-Id: I9427e025ea0bcf2fa24fd539a775429cc64767cc
2019-11-29 13:40:34 +01:00
Adam Wight ab78df8d5c Wire extends into ReferenceStack
Takes no action, just shuttle the value between functions.

(Split from I9427e025ea0)

Change-Id: I271043e9161835f3278098787bf58b50ed93c892
2019-11-28 02:10:11 +01:00
Thiemo Kreuz a6a16f0703 Update and increase ReferenceStack test coverage
Functional changes:
* hasGroup() will return false when a group exists, but is empty. This
  is in line with what other methods like getGroups() already do.
  Shouldn't have any effect on the existing code, but feels more clean
  and consistent.
* getGroupRefs() won't fail any more when asked for an unknown group.

Tests:
* Add missing @covers for the constructor.
* Simplify test setup by always returning a spy. All tests need it
  anyway.
* Cover 3 more methods.

Change-Id: Ie93e9af6258b757d842b30b0b059344733aad434
2019-11-27 11:08:00 +01:00
Adam Wight ec091fe906 Reorder keys
This doesn't make any functional difference, but helps minimize later
patch Ida9612d14

Change-Id: Ice89bad02e077437d0df6fa9f51f90b4cab4837c
2019-11-26 17:06:01 +01:00
Adam Wight 890e86a7fb Fix tests: cannot have name and follow
This was impossible and is prevented by validation, so do not test.

Change-Id: I836b38c700f41f692e5c6a893be0076febfc9c4d
2019-11-26 16:57:56 +01:00
Adam Wight 55099a7b0c Remove impossible condition
Numeric `$name` is caught during validation.

Change-Id: Id1c3e6717af38b0b1393c135732e084d261b53f6
2019-11-26 16:43:21 +01:00
Adam Wight 8453e3ecd7 Extract stack and state to a new class
Most of this state is used to manage interactions with other state,
and encapsulation allows us to hide data structures and access behind
self-explanatory function names.

The interface is still much wider than I'd like, but it can be improved in
future work.

There is one small behavior change in here: in the `follows` edge case
demonstrated by I3bdf26fd14, we prepend if the splice point cannot be
used because it has a non-numeric key.  I believe this was the original
intention of the logic, and is how the numeric case behaves.  I've verified
that when array_splice throws a warning about non-numeric key, it fails to
add anything to the original array, so the broken follows ref disappeared.

Bug: T237241
Change-Id: I091a0b71ee9aa78e841c2e328018e886a7217715
2019-11-25 14:06:32 +01:00