Commit graph

6 commits

Author SHA1 Message Date
Thiemo Kreuz d80bd3ef97 Rewrite ErrorReporter for performance and separation of concerns
This patch is mostly moving code around without changing the behavior.
Exceptions:

* The ErrorReporter creates a <span> container. This was previously
parsed. The only benefit might be error checking and escaping. Rather
pointless. The code just created this HTML. With this patch, it is not
parsed any more. The unit test reflects this change. The output in
production will not change, as the parser tests show.

* Parsing of the message key (to detect it's type and id) is simplified
a lot, using explode. With this the code can, in theory, support more
types.

Bug: T239572
Change-Id: If2fe5f55db46dfc7e0ce445348608bef00bec64e
2020-02-03 15:23:40 +01:00
Thiemo Kreuz 013e1bfa90 Final clean-ups for a more consistent parameter order
* Always have an empty line between @param and @return to improve
readability as well as consistency within this codebase (before, both
styles have been used).

* Flip parameter order in validateRefInReferences() for consistency with
the rest of the code.

* In Cite::guardedRef() the Parser was now the 1st parameter. I changed
all related functions the same way to make the code less surprising.

* Same in CiteUnitTest. This is really just the @dataProvider. But I feel
it's still helpful to have the arguments in the same order everywhere, if
possible.

* Add a few strict type hints.

* It seems the preferred style for PHP7 return types is `… ) : string {`
with a space before the `:`. There is currently no PHPCS sniff for this.
However, I think this codebase should be consistent, one way or the other.

Change-Id: I91d232be727afd26ff20526ab4ef63aa5ba6bacf
2020-01-09 12:13:54 +01:00
Thiemo Kreuz 446524f8a9 Consistent empty lines between @param and @return PHPDoc tags
In the end I don't care much if we agree on having this newline, or
not. What I care about more is that this codebase is consistent.

Personally I prefer having the newline. It creates a visible separation
between what "goes in" and what "goes out" (@throws and @return).

Change-Id: Ibc60af621132e415a5579397c01688fa21eb0be5
2020-01-09 12:00:35 +01:00
Adam Wight 9fd825e6a6 Simplify null comparison
In these cases, an expression is either true-ish or null, so we can use
the implicit boolean cast to test.

Change-Id: Ibe94829f9774bf2a1907635a8bd28369908b4d1e
2019-12-12 16:42:53 +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
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
Renamed from src/CiteErrorReporter.php (Browse further)