Commit graph

18 commits

Author SHA1 Message Date
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
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
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
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
Renamed from includes/Cite.php (Browse further)