It appears like this was never tested.
Now that it is covered it's much easier to play around with the
implementation and compact it a bit.
Change-Id: Ie9cc14082f69e7240380d352fb362d0a3fa4d341
Before the (intentional) design decision was to not do anything special
when the same parameter is used multiple times (via aliases). Garbage
in, garbage out. Only the first usage of the parameter would work as
intended. The rest was ignored and subsequently removed from the
wikitext.
New design decision: Track and display duplicates as they appear in the
wikitext.
Notes:
* It's not possible to create such a situation in VE. Do this via
wikitext.
* Labels will be made distinguishable via T309198.
* Possible warning messages will be added later.
* The behavior when unchecking a duplicate will be specified later.
Bug: T309198
Bug: T310248
Change-Id: I6011344638cdad8529d8f57513ef51b5237eb878
The numeric part of these ids is never used on it's own. There is no
need to expose it.
Note we renamed the method not long ago in I6eeab8b to reflect better
what it does. This is the next step. We just forgot it back then.
Change-Id: I5da82855e99ea3a42a5d91379c6974ae9c154518
These are only needed when we need to access a specific `this` from
within another `function () {}` context. This is not the case in the
situations here.
This is split from Ibf25d7e to make it smaller and easier to argue
about.
Change-Id: Ide1476de91fc343aa992ad92a1321d3a38b06dd0
It's not a getter, but a generator. I found the name confusing.
Getters typically don't return something different every time you
call them.
Change-Id: I6eeab8b6a8644e430003f6e1ad77ab4b28e0d8c9
When I type e.g. "subst:example" as the template name, we made this
work as the user would expect: the template named "Example" is found
and it's TemplateData documentation used. But the dialog title shows
"Subst:example". Note the uppercase "S". It means this string is
parsed as a title, including the "subst:". This is confusing. Just
show the template name.
Change-Id: I9817786991a8379cf48b0a664aef1413abddee2d
I was (again) wondering why the try-catch is there. But it's actually
needed. Now covered by a test.
Bug: T290140
Bug: T291062
Change-Id: Iccc7274ed9e7b81b8491dbca7d3d771706b0ed09
* The extra brackets are not needed in ES6.
* Make sure the name of the test is on the next line. This makes
the code easier to read.
Change-Id: Ib871dbfa27fcadc88e7335b9efb4d583bd4300ac
The code in .cacheTemplateDataApiResponse() where the `specCache`
is filled skips missing pages. .setTemplateData() is never called.
While we could – in theory – check the `missing` flag (as done in
patchset 1), this flag never makes it to the spec.
Rather simple solution: Mark everything as undocumented, as long
as .setTemplateData() is not called.
This affects only missing pages. .setTemplateData() is called in
all other situations.
Bug: T272487
Bug: T276574
Bug: T290136
Change-Id: I7045e84f2f2ba5aa4591c94ea495b0249e6c40d6
These tests obviously don't need this extra environment.
They run just fine (and faster) without.
Bug: T289560
Change-Id: Ib186a07cd556f741e0440ffa54ae6aaaf626adcd
Separation of concerns:
* The template model knows which parameters are currently used,
but doesn't know what's documented.
* The spec knows what's documented, but doesn't know what's
currently used.
Change-Id: I97cac00d6775a17a07059d0e8a7a116adc6080b3
If a known parameter is present using one of it's aliases, then
only the aliased name should be shown to the user. This patch,
therefore, resolves the issue of the same parameter being added
to the sidebar twice.
When adding a parameter that is aliased, it will receive the same
position as the non-aliased parameter it is replacing.
Bug: T274545
Change-Id: If4e58c941fd0f0e690d3603935f5a5d3f9938163
This fixes a minor issue in the spec class. In the first step,
parameters from the template are added to the list of known
parameters. Later, aliases are resolved. The original behavior
was that such a parameter moved to the end of the list. This
is rather unexpected.
This dosn't have much of an impact. The pretty much only place
where the parameter order from the spec can be seen is in the
parameter search widget. Still I believe it's worth fixing.
Bug: T285483
Change-Id: I455818451811e92bba3e9320c2d41e1db8d563f2
I don't want this code to crash when the TemplateData API
returns an unexpected result.
Bug: T285483
Change-Id: I237cbfbb85892a53a08d9e7e34cf4974775d627a
This doesn't "extend". It was never used like this. What it
actually does is to link between a (cached) TemplateData blob
and the spec class that want's to use it.
Is this the best possible name?
* fillFromTemplateData( … )?
* propagateTemplateDocumentation( … )?
* readDocumentationFrom( templateData )?
* …?
Do we want to rename the "spec" class as well?
* MWTemplateDocumentation?
* MWTemplateMetadata?
* MWTemplateDataAccessor?
* …?
Bug: T285483
Change-Id: I6c52ef42d411c2f47fc0080768d36ebda4dd2a55
The so called "spec" class keeps track of parameters that have
been used before, no matter if documented via TemplateData or
not. Removed parameters are still "known" (i.e. have been seen
before).
This feature allows to easily find previously used parameters
names when an undocumented parameter was removed and the user
tries to add it again.
Bug: T285483
Change-Id: Ia1555eea87cd99e7a3f386f4279ec5a80fb98a79
In I04b8a14fbec7be5a1c4defabf92e94f694c1e638 we sepearted params from
aliases. There we missed that re-filling the parameters from the
template could re-add the aliases.
Bug: T285483
Bug: T285843
Change-Id: I1928b443a5f708bc8c57efa5ad0a86b5915b159c
While the term "canonical" is not wrong, I find it still
somewhat ambiguous.
1. "Canonical" could mean different things. E.g. is the order
of parameters as they appear in the article's wikitext the
"canonical" one? It's possible to argue like this, esp. if a
template doesn't have TemplateData documentation. In this case
the only order known is the one from the wikitext.
2. "Canonical" sounds like the parameters must be reordered.
But this should never happen. Not having dirty diffs is more
important than having the parameters in a specific order.
Bug: T285483
Change-Id: I23658d37fea50b727667677ac6a49066673b2135
Wait, what's going on here? This patch looks like it changes the
behavior of this code. But it doesn't. Here is what happened
before:
* Let's say a template contains 2 parameters, A and B.
* We don't know yet if these names are aliases.
* getParameterNames() returns [ "A", "B" ].
* extend() is called. The TemplateData documentation contains
the parameters "B" and "C". "C" does have an alias "A".
* extend() can't find "C" and adds it to the end, as if it's a
new parameter.
* extend() also iterates the aliases. For each alias it creates
a reference to the specification object. In this case a
reference from "A" to "C" is created.
* But "A" already exists. The position of "A" doesn't change,
but the specification now says it's an alias.
* getParameterNames() skips aliases. It skips "A" and instead
returns the new "C" from the end of the list.
This was the behavior before. It's unchanged, proven by the tests.
Change-Id: I04b8a14fbec7be5a1c4defabf92e94f694c1e638
The idea is to not actually store all these default values, but
fall back to the default only when needed.
Some more details:
* The only remaining property is ….name. The only reason to
have this property is to distinguish between aliases and
primary parameter names. This will be reworked in a later
patch.
* The description falls back to null because this is the
documented fallback, not undefined.
* The default value falls back to "", same as the auto-value.
Why not null you might ask. This is intentional. Both the
auto- and default value are effectively wikitext snippets,
while the example is a label in the VE UI.
Bug: T285483
Change-Id: I1be3cca18f9ad6fc1c16362b24633f7613f02539
This is done for two reasons:
1. It fixes the behavior of two methods in rare edge-case
situations. They aren't documented to return undefined.
2. It reduces the amount of stuff this class stores when it's
nothing but a default value anyway. Note this patch does this
for the template-level properties only. Another patch will do
the same for the parameter-level properties.
Bug: T285483
Change-Id: If2e4d56da1fa52e32dc94191f36d7dc6a1487829
This reflects much better how this method is meant to behave.
Note I will continue to remove documentation that doesn't
explain anything in addition to what the code already says.
Bug: T285483
Change-Id: I81fa8a5d9d0752f3aeac4015c9a27b50e054d4df
This patch also marks 2 methods as @private that are not and
should not be used outside of this class.
Bug: T285483
Change-Id: I8a8ffc4868a369b5c47068beb0e83f023872543d
