Commit graph

78 commits

Author SHA1 Message Date
Thiemo Kreuz 54333c9bd6 Stop formatting "1.2" as "1,2" in languages like German
The new extends="…" feature is using numbers like "1.2". These should be
localized in languages like Hebrew that uses other symbols for the digits.
But the "." should not change.

The existing feature when a <ref> is reused multiple times does have the
same "issue". But it seems this is intentional, because it is covered by
a test. Note this is not visible in German, because German uses custom
labels "a", "b", and so on.

This patch also improves the so called "smoke" tests and makes one cover
numbers up to "1,10" for a <ref> that is reused that often.

Bug: T239725
Change-Id: Iffcb56e1c7be09cefed9dabb1d6391eb6ad995ce
2019-12-04 09:43:04 +01:00
Adam Wight 1b82b93835 Fix function signature in phpdoc
Change-Id: I3329ca19d465c6ad7ed23385021a051fcc23ea8e
2019-12-03 13:26:50 +01:00
jenkins-bot 0f9c306748 Merge "Inline and streamline code in the formatter classes" 2019-12-03 10:42:28 +00:00
Thiemo Kreuz b145869980 Inline and streamline code in the formatter classes
* Don't use string comparisons to compare numbers.
* Avoid isset() for variables that are guaranteed to exist.
* Inline two small "gen…" functions that are only called once.
* Move the fallback code path out of getLinkLabel(). Before it was
  always called. Now it's only called when needed.

Change-Id: I42073f57f21d32c7936954da776ef3a393410020
2019-12-03 10:26:38 +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 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 a27c33a2e7 Cleanups
Address some code review comments from I75bd6644

Change-Id: I433c08318c137ecca4d4ef77f0863d5da42b567c
2019-12-02 16:36:49 +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 00f3be7c7f Reset extendsCount after each group
This was a bug which would affect book references, if the same group
and parent ref name combination occur twice in an article.

Change-Id: I608f58aac0cec31c8650835fc80195a87bc851d3
2019-12-02 10:17:18 +01:00
Thiemo Kreuz 504db2c46a Add strict PHP 7 type hints to most code
This patch does two things:
* Add strict PHP 7 type hints to most code.
* Narrow the interface of the checkRefsNoReferences() method to not
  require a ParserOptions object any more.

Change-Id: I91c6a2d9b76915d7677a3f735ee8e054c898fcc5
2019-12-02 08:51:42 +01:00
jenkins-bot 469a1e6364 Merge "Don't leave unclosed <li> behind" 2019-12-01 10:36:56 +00:00
jenkins-bot 76ef4af51f Merge "Clean up text and name conditionals" 2019-12-01 10:02:41 +00:00
Thiemo Kreuz 3cf1e99cc2 Don't leave unclosed <li> behind
This fixes a FIXME I left in the code. Previously, I just stripped the
closing </li> to make sure the nested <ol> is *inside* of the <li>.
This relies on (Remex) Tidy to clean the incomplete HTML up.

This patch remembers the stripped </li> and adds it back.

This also makes sure the nested <ol> is closed, even if it was the
last element in the data structure.

Notice how this does not influence any test. I find this a bit
confusing. It looks like (Remex) Tidy is executed, even if the tests
are not marked as "html/php+tidy".

Bug: T237241
Change-Id: Idb804df46dc24406d6bba40414675b6ff4812d48
2019-12-01 10:54:01 +01:00
Adam Wight f24f77d4c4 Fix comments
Change-Id: Ie99f172bf555af4e2c51928152043d12ea735d76
2019-11-29 23:01:07 +01:00
Adam Wight 6922f5201b Drop single-use variable
Change-Id: Idb9801cc1f414088841afc2f18a056be65b695d1
2019-11-29 23:01:06 +01:00
Adam Wight ffedf86a19 Clean up text and name conditionals
The "no key" error should have been unreachable, but I'm afraid that
null `$text` and empty string `$text` were reporting slightly different errors.

Unfortunately, we still have to care about `$text = '0'` because PHP, so
the expressions don't reduce to `if ( !$text ...`

Change-Id: Id1028611ec3bc462dca413f31f7f59637bd7cc7b
2019-11-29 19:26:12 +00:00
Thiemo Kreuz 22627f074d Make the normalizeKey() method private
There was a call in the API that was *not* using normalizeKey(). Now
that the API is gone, we can inline this.

This patch also contains a bunch of cleanups that might already been
resolved in the previous patches.

Change-Id: Id3767b5830268c8cfe9c10efabfa4a31e9dafeb8
2019-11-29 19:02:48 +01:00
Adam Wight 367de442c1 Move string contatenation out of sprintf
Simplifies what's actually happening here.

Change-Id: I7b561355506c1f4aa757cf551aa859a32fe23567
2019-11-29 16:22:35 +01:00
Adam Wight a40b1b10be Extract footnote body rendering
Change-Id: I9537849cbd700d5dc7ec1a53d852d69b0fe0dc35
2019-11-29 16:22:35 +01:00
Adam Wight c236524138 Extract footnote mark rendering
Change-Id: I79de89e46da36dc1f0ee2b2fdb9a139e6434fde2
2019-11-29 16:22:35 +01:00
jenkins-bot 4b58a25459 Merge "Extract key formatting" 2019-11-29 15:19:52 +00:00
jenkins-bot 50ccd05def Merge "Render nested references" 2019-11-29 15:19:50 +00:00
Thiemo Kreuz f8affe8eee Remove redundant variable names from @var docs
Change-Id: I5ba2f1041d3d6770c58f496482d2799bb24786be
2019-11-29 14:37:58 +01:00
Adam Wight a4c056f59b Extract key formatting
Change-Id: I155ec6f3e21075587dbcfdfdc346f28f958e3c15
2019-11-29 13:41:12 +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
jenkins-bot 3beb5c3634 Merge "Remove ApiQueryReferences support" 2019-11-29 11:30:38 +00:00
Adam Wight a176e22097 Remove ApiQueryReferences support
This API was never used in Wikimedia production, and would have caused
performance problems.  Removing the dead code will simplify our refactoring.

Bug: T238195
Change-Id: I7088f257ec034c0d089e0abdaa5a739910598300
2019-11-28 11:08:46 +01:00
Adam Wight 44cbc60d40 Count extends refs
Has no effect, this is a "safe" split from I9427e025ea0.

Change-Id: I842673cd1226ec5c9248d8f069766a00a7c27f35
2019-11-28 02:10:28 +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
jenkins-bot dbf4c56896 Merge "[Refactor] Pass validation error with StatusValue" 2019-11-27 21:23:30 +00:00
jenkins-bot e4a961a6c1 Merge "Rename $valid to $status for clarity" 2019-11-27 21:23:29 +00:00
Adam Wight 249982e353 Rewrite loop in ReferenceStack
Change-Id: I3bdf26fd14573abdcad989c7ebfea48e49ef42aa
2019-11-27 17:13:44 +00:00
Thiemo Kreuz f00b21943b Minor fixups to the ReferenceStack class
Change-Id: Ie7d72b13f987443c0e118fb9ac0f0af016f00392
2019-11-27 18:13:22 +01:00
Adam Wight 22a0350d84 [Refactor] Pass validation error with StatusValue
This has clearer semantics than checking for a `false` attribute.

Change-Id: I68f777eda40f8f157deafacaed02d4bd10cbf25c
2019-11-27 18:05:19 +01:00
Thiemo Kreuz 99ee9e443b Rename $valid to $status for clarity
This also splits some code a little bit to make the next patch smaller.

Change-Id: Ibc02fa3d683043de86d21a7aa3feef373502552a
2019-11-27 17:51:22 +01:00
Adam Wight b30340ba2b Clean up pushRef
This is motivated by I9427e025e, which demonstrates that the existing
logic was hard to integrate into.  There's a lot of redundant expressions
which make the function difficult to read, and code paths which have
less effect than they appear to.

Change-Id: Ida9612d1457f2593647b8fc02930d2e9ae824814
2019-11-27 16:40:51 +01:00
Thiemo Kreuz 0013943a4a Rewrite argument parsing and use for both <ref> & <references>
We realized the trim() are not needed. This does not leave much behind
in the existing refArg() method, except that it checks for unknown keys.

I tried a few strategies and ended using the pretty new possibility to
have keys in list(), as well as use [] instead of list(). Both is
supported since PHP 7.1.

Change-Id: I569bfa14e68b64402519bd39022c197553881dde
2019-11-27 14:01:52 +01:00
jenkins-bot 7ed54a3f3d Merge "Remove redundant attribute trimming" 2019-11-27 12:26:00 +00:00
Thiemo Kreuz 99d23ac841 Remove redundant attribute trimming
We noticed the group="…" attribute was the only one that was not
trimmed. Does this mean it was possible to have two groups "a" and
" a"? It turns out: no. This was never possible because the parser
already trims all attributes before calling this code.

I tried to come up with the worst possible test case, but it succeeds,
even with very old versions of this codebase.

I suggest to remove the extra trimming from this codebase and rely on
what the parser provides.

Note the content is special and *not* trimmed by default.

Change-Id: Idff015447d7156ba7b5c03a5c423f199a71349f2
2019-11-27 12:12:51 +00:00
Thiemo Kreuz 04f784bc02 Remove non-existing property from ReferenceStack
Change-Id: Id789897dd92c7692e36f54b828c097820ab46b43
2019-11-27 12:57:55 +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 97746db93c More specific function signature
Change-Id: Id1d21b9abdb11bbf441650ea0a1cccc8b258d598
2019-11-26 18:27:16 +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 38d7c09495 Tweak comment
This makes it show up nicely in my IDE.

Change-Id: Ic203a9dcb83c96c3324996e183b4dfc239f65eca
2019-11-26 17:03:40 +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 7cdcc2b075 Alphabetize returned array of attributes
That was annoying me.  Since we're passing a bare list, alphabetical
order helps make the code and tests readable.

Change-Id: I6384094e429e0e2a6fa810fdc28ae0643a0ccf7c
2019-11-26 15:28:02 +01:00
Adam Wight 6a9e51fc30 Tag every usage of "extends" even when invalid
We weren't recording usages which failed certain `refArg` validations.

Bug: T237531
Change-Id: Ibcb875c5d0ed6c2279e0e34ab415ac63d7ebe584
2019-11-26 15:28:02 +01:00
Adam Wight 2229f22899 [Refactor] Nest conditionals with common term
Previewing is an important exception, so I wanted to consolidate and
emphasize its edge cases.

Change-Id: Iae343ed8c225407e8184ff09e426d531c9f6ab00
2019-11-26 15:28:02 +01:00