Commit graph

3212 commits

Author SHA1 Message Date
jenkins-bot 71f6b91964 Merge "Add some comments" 2019-11-26 14:22:50 +00:00
jenkins-bot b78cdd888d Merge "Signature: these are all nullable" 2019-11-26 14:22:50 +00:00
jenkins-bot 68ade8af25 Merge "Drop unused param" 2019-11-26 14:22:49 +00:00
jenkins-bot 626e6ab00c Merge "Comments in TODOs" 2019-11-26 14:22:48 +00:00
jenkins-bot dfb0ccdac6 Merge "Test for numeric extends attribute" 2019-11-26 14:22:39 +00:00
jenkins-bot ae28a12e31 Merge "Test for numeric attributes earlier" 2019-11-26 14:20:57 +00:00
jenkins-bot 830755d5fb Merge "[Refactor] Consolidate validation" 2019-11-26 14:19:44 +00:00
Translation updater bot 7b92fe355a Localisation updates from https://translatewiki.net.
Change-Id: I71d3b80d66e9ed3a3f38d24b00064fe4aa472818
2019-11-26 11:42:18 +01:00
Adam Wight d1e0e6309b Add some comments
Change-Id: I012ad6cc52dc65f1b329febdc8e6441ac03c6463
2019-11-26 11:07:48 +01:00
Adam Wight 9fa22b15be Signature: these are all nullable
Change-Id: I79432de9007f9016604cb871b68577f1cf596be5
2019-11-26 11:07:39 +01:00
Adam Wight d65bb667b7 Drop unused param
The "dir" param is not validated, so don't pass through.

Change-Id: I78eaa3bf7067c6283a2d8e452b68f93ffab43875
2019-11-26 11:07:39 +01:00
Adam Wight 33014bb8f5 Comments in TODOs
Stuff we have to fix in future work.  Follows up on I2d9904b7631d0d6

Change-Id: I7bca73d3e9d3d2a224604efbe81d48948d2a3d76
2019-11-26 11:07:12 +01:00
Adam Wight 22b0bdf526 Test for numeric extends attribute
This is invalid, because it would allow access to internal, autoindexed,
anonymous refs.  These would break when refs are reordered.

Bug: T151305
Change-Id: Ib4bb8270d810b64e4c160f377ce52ce2fc70bab4
2019-11-26 11:07:11 +01:00
Adam Wight feaa724efa Test for numeric attributes earlier
This introduces a slight behavior change, but for the better:
* When pointing to the name "0", the non-numeric error will be displayed,
  which is correct whereas "no key" is not.

Change-Id: I33467b27cd447812fe67204831909c4d9869db08
2019-11-26 11:06:15 +01:00
Adam Wight 3fbeed0304 [Refactor] Consolidate validation
Validation logic can be split from arg parsing, default values and
other side-effects.

No behavior was changed.

Change-Id: I2d9904b7631d0d6be13e0aaed0106f186d388c4f
2019-11-26 09:25:06 +01:00
Translation updater bot 1d2885cd3e Localisation updates from https://translatewiki.net.
Change-Id: I9c658c7036b90e4d67b434ff98dedc396617dbee
2019-11-26 09:16:52 +01:00
Adam Wight 9e2468882d Cache parser previewing state
We need to access this in several places, store as instance state for
convenience.

Change-Id: I4ea8f279a34cd8f819d9c07c75e3e8e160786f9b
2019-11-25 14:06:55 +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
Adam Wight 10e4a4353d Finish renaming "key" to "name"
Change-Id: Iffd49268cfefefdce9c970f60b5d231e79cbc267
2019-11-25 13:25:40 +01:00
Adam Wight a0f019e1ac Clean up variable names in two more functions
Change-Id: I7e0eb97123a53463133226ae2067d2396e8ceda3
2019-11-25 12:39:57 +01:00
Adam Wight 3ec5a7c3e7 Clean up comments
Clarify where different keys come from and what they're used for.

Change-Id: I534de4952c5b0e053dcba95f31c837547b4a68a6
2019-11-25 12:37:07 +01:00
Adam Wight b9c51b81a1 Rename variable to $name
This is still directly fromt the "name" attribute.

Change-Id: I2c3d0faf591be7e5032c0b26cf5eb7542390bd64
2019-11-25 12:35:42 +01:00
Adam Wight 17c5d6e981 Use "name" to be consistent with attributes
Call the variable the same thing as the attribute it comes from.

Change-Id: I012e29018bdabadc7ec87ed13fc396a7a653933e
2019-11-25 12:31:32 +01:00
Adam Wight fb430f257e Fix variadic parameters in error reporter
Give these functions the same signature as `->msg()`

Change-Id: Ib90df52d6752512d7d9dddf51777c9c23c847e06
2019-11-25 11:02:31 +00:00
Translation updater bot 683b4e3e6b Localisation updates from https://translatewiki.net.
Change-Id: I238c18b40b4d12376b6055efa0bc5b6eca278a59
2019-11-25 09:09:29 +01:00
Thiemo Kreuz (WMDE) 5401fcd190 Revert a part of "Add missing test cases for follow="…""
This partly reverts commit 8e42a6ecdf.

The variabe $k as created by the foreach() loop is not necessarily
numeric, because the $this->mRef structure contains data both for
named and unnamed <ref>s. The array key is a (non-numeric) string for
named, and an integer for unnamed <ref>s.

array_splice() requires a position, not an array key.

Note that both implementations are wrong. The foreach() might return a
string $k, which makes array_splice() complain and do unwanted things.
The for() loop assumes there are count() array elements with integer
keys, which might not be true. Luckily this was not a problem, because
the isset() check would stop the (to long) loop eary enough.

A better rewrite as well as a test case for this will be added with
I3bdf26f.

Change-Id: I5568d3084197f1861f9dc8983d8b606a961e201f
2019-11-23 22:11:17 +01:00
Subramanya Sastry 2cfb76f8b6 Sync up with Parsoid citeParserTests.txt
This now aligns with Parsoid commit 7dfc2e931a6afeb62d2a0d791cda88fd8d39c070

Change-Id: I7edd1f293530653ae1bbfe47028e585f2b46927b
2019-11-22 18:44:22 +00:00
Thiemo Kreuz c76a5e84f9 Fix misleading method names in CiteErrorReporter
I realized especially the method name html() was wrong. It does not
return HTML. What it returns is still wikitext and must still be parsed.
It only applies some early steps of the parsing process, e.g. expanding
extension <tags>.

Change-Id: I2c403a77eef843940f34f0933e4bfe58e6200ce5
2019-11-22 15:08:39 +01:00
Thiemo Kreuz 177c9cc1eb Fix inconsistencies and deep nesting for follow="…"
* This fixes the refArg() function. If there is nothing wrong with the
follow="…" attribute, it should not return null.

* However, *everything* is false if an unknown error (e.g. an unknown
attribute) occurs.

* A trivial check for `if ( $follow )` is fine because all keys are
guaranteed to not be the string "0".

Change-Id: Ia4e37781e01db1ee6615ffc30bb68e47023c6634
2019-11-22 15:01:09 +01:00
Thiemo Kreuz a823fa23d9 Merge two more code paths in Cite::referencesFormatEntry()
There was another, duplicated special case for previews. It was using
the same message as a <ref> with multiple uses. Now it's only one code
path.

The goal here is to reduce the number of code paths to make it much
easier to implement proper rendering for the extends="…" use cases.

Bug: T237241
Change-Id: I863ac3b5234d3a6f7f2371a2a85385c3aea276e5
2019-11-22 15:01:09 +01:00
jenkins-bot 7f4cff9523 Merge "Move bad dir="…" error reporting down to the renderer" 2019-11-22 13:46:44 +00:00
jenkins-bot 15985a7fa7 Merge "Fix internal presentation of the dir="…" attribute" 2019-11-22 13:26:49 +00:00
Thiemo Kreuz 8e42a6ecdf Add missing test cases for follow="…"
One of the test cases was duplicated, but a lot of the possible code
paths never had tests, including the happy code path!

I found this issue while trying to rework some of the more confusing
loops in this codebase. These changes are still part of this patch. All
loops still do the same as before, but are (I hope) more readable now.

Bug: T238187
Change-Id: I85baeadd9b149025a14c7522bcc4182339c66972
2019-11-22 11:32:28 +01:00
Thiemo Kreuz 55707a745e Rewrite Cite::inReferencesGuardedRef()
Change-Id: I74960a92b3530ef97565cd2e2f79e9696e97f975
2019-11-22 10:14:42 +01:00
Thiemo Kreuz ea6cea93ed Move bad dir="…" error reporting down to the renderer
… and make the error message for bad dir="…" shorter and more to the
point.

Now I understand why the error reporting was not done when $text was
empty: the error was actually appended to $text, which messes with
everything else that also works with the $text variable! This even
includes the API. This error message was exposed via the API. That was
certainly a bug.

With this patch, all error checking for the dir="…" attribute is now
done way down, when rendering the <references> section.

Note this also fixes a bug where the dir="…" was *not* rendered when
previewing a section.

Change-Id: I4ab0cb510973ed879c606bfaa394aacc91129854
2019-11-22 10:07:28 +01:00
Translation updater bot b01f9f67a4 Localisation updates from https://translatewiki.net.
Change-Id: Icf0e48075c50d682d4bdc4330be1816b65939de9
2019-11-22 09:13:52 +01:00
Thiemo Kreuz 65c8967c32 Fix internal presentation of the dir="…" attribute
This fixes a whole bunch of inconsistencies:

* The dir attribute is now trimmed, as most others already are. This is
an actual user-facing change.

* The internal representation is now false in case the value was invalid,
not an empty string any more.

* Null means the attribute was not present. This is now always used,
even in the return values that are meant to represent an error state. No
existing behavior changes.

* The internal representation does not contain an HTML snippet any more,
but the raw value "ltr" or "rtl", or null. Note this might influence the
API, because the API actually exposes the internal representation.
However, we are pretty sure the API is not used anywhere. Even if,
exposing HTML code was most certainly an unwanted and unexpected effect
of the patch that introduced the dir attribute. This does make this a
bugfix, I would argue.

Change-Id: Ic385d9ab36fa0545c374d3d63063028ae4e449d4
2019-11-21 12:52:47 +01:00
Thiemo Kreuz ab3063fee5 Move all code to PSR-4 compatible namespaces
This patch does intentionally not touch any file name. Some of the
file names are a little weird now, e.g. \Cite\Cite. These can more
easily be renamed in later patches.

I used https://codesearch.wmflabs.org/search/?q=new%20Cite%5C( and it
looks like this code is not used anywhere else.

Change-Id: I5f93a224e9cacf45b7a0d68c216a78723364dd96
2019-11-20 17:00:13 +01:00
jenkins-bot cf6e2baeab Merge "Minimize paths in all RL modules utilizing localBasePath" 2019-11-20 15:51:41 +00:00
Thiemo Kreuz b10dd4ec27 Block de-facto empty <ref> as if it's empty
The use case we care about is this:
<ref extends="some_book"> </ref>

It doesn't make sense that works, but the following doesn't:
<ref extends="some_book"></ref>

We decided that both need to behave the same.

For consistency this patch is applying the same change to all references,
no matter if they use the extends attribute or not. This is an actual
change and might make existing wikitext render differently. However, I
would like to argue that all wikitext that was using this was broken. The
effect of a <ref> </ref> with some whitespace is that the <references>
section at the end of the article will contain – well – an empty footnote.

Bug: T237241
Change-Id: Iaee35583eabcb416b0a06849b89ebbfb0fb7fef9
2019-11-20 15:07:54 +00:00
Thiemo Kreuz a93442f0b4 Minimize paths in all RL modules utilizing localBasePath
Change-Id: I484849ad7609822c4fafbe12b7737e0c715eb8ee
2019-11-20 15:02:49 +00:00
Thiemo Kreuz 8e800a8988 Merge two <references> related properties into one
Change-Id: Idb6c220870e8d76a7d011f5d0b3e8b8054f2d867
2019-11-20 10:31:03 +01:00
jenkins-bot 32e1f8e7c3 Merge "Don't pass a Title object around that's not needed" 2019-11-19 16:09:13 +00:00
jenkins-bot 7018e82352 Merge "Extract all error reporting to a CiteErrorReporter" 2019-11-19 15:53:29 +00:00
jenkins-bot 6f8b7311d9 Merge "Fix bad settings name CiteCacheReferencesDataOnParse" 2019-11-19 15:53:00 +00:00
jenkins-bot c5bbed5928 Merge "Get rid of obsolete method_exists() checks" 2019-11-19 15:49:40 +00:00
Thiemo Kreuz 9d2d61ff09 Don't pass a Title object around that's not needed
Change-Id: Iea9c366c4b45ba4cd9171c8b4fffc307c852b6e2
2019-11-19 16:48:36 +01:00
Thiemo Kreuz 7c1e098b7c Fix bad settings name CiteCacheReferencesDataOnParse
* CiteCacheReferencesDataOnParse doesn't exist in any code.
* CiteCacheRawReferencesOnParse does.

Change-Id: Idc65d483cf5b7a7b25fd0aed7915c9c0958611f0
2019-11-19 16:13:07 +01:00
jenkins-bot e389e3c1bb Merge "Make two Cite properties private" 2019-11-19 14:54:11 +00:00
Thiemo Kreuz deca410d59 Get rid of obsolete method_exists() checks
Change-Id: Ic3396537a8bf5dc39fd3bea089ebd2292e1c6be5
2019-11-19 15:31:28 +01:00