Commit graph

21 commits

Author SHA1 Message Date
Thiemo Kreuz 3cd91100a9 Move code that belongs to the template specification
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
2021-07-07 16:45:35 +02:00
jenkins-bot 4a5f811c82 Merge "Keep template parameter position when resolving aliases" 2021-07-05 11:44:19 +00:00
Andrew Kostka 6af13f0d42 Fix parameter ordering when using aliases
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
2021-07-05 08:14:42 +00:00
Thiemo Kreuz 8d0a623f40 Keep template parameter position when resolving aliases
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
2021-07-03 20:14:06 +02:00
Thiemo Kreuz 280ba23e2c Add missing fail-safe to MWTemplateSpecModel.extend()
I don't want this code to crash when the TemplateData API
returns an unexpected result.

Bug: T285483
Change-Id: I237cbfbb85892a53a08d9e7e34cf4974775d627a
2021-07-02 17:52:43 +02:00
Thiemo Kreuz 07344fce0f Rename misleading "extend" method in template spec class
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
2021-07-02 11:26:04 +00:00
jenkins-bot 2e4ac455f0 Merge "Add more edge-case test cases for template spec class" 2021-07-01 18:57:51 +00:00
Thiemo Kreuz 99523b855c Use and document the term "known parameter" in template code
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
2021-07-01 12:03:38 +00:00
Thiemo Kreuz d8f4da199d Add more edge-case test cases for template spec class
Bug: T285483
Change-Id: I0a9bf7fe51242161dbc57c9301f360c9e8f35b93
2021-07-01 10:13:00 +02:00
jenkins-bot 127ea9ea27 Merge "Avoid the term "canonical order" in template related docs" 2021-06-30 11:35:27 +00:00
jenkins-bot 5943b5a25e Merge "Fix spec.fillFromTemplate() not skipping aliases any more" 2021-06-30 11:11:24 +00:00
Thiemo Kreuz 84329a40c8 Rename temporary "x" variable in spec class tests
I forgot to give this variable a proper name. The "x" was not
meant to stay.

Bug: T285483
Change-Id: I482b9282676082f44d5f98a574e5ada05dfb6318
2021-06-30 12:16:50 +02:00
Thiemo Kreuz 466e428a81 Fix spec.fillFromTemplate() not skipping aliases any more
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
2021-06-30 10:14:43 +00:00
Thiemo Kreuz bc4aeed86e Avoid the term "canonical order" in template related docs
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
2021-06-30 09:38:28 +02:00
Thiemo Kreuz b7d88c541b Fix all spec methods to not crash on unknown parameters
Change-Id: I16708b048a785f0712084bd2d087e4aab77fb72b
2021-06-25 08:36:23 +02:00
Thiemo Kreuz b8382513fc Use separate data structure to store aliases in spec
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
2021-06-25 08:29:48 +02:00
Thiemo Kreuz 8f6098c03b Simplify spec code dealing with default values
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
2021-06-24 18:13:41 +02:00
Thiemo Kreuz bab08440ea Fix getDescription/Sets possibly returning undefined
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
2021-06-24 18:07:46 +02:00
Thiemo Kreuz 2529d33f51 Better name for spec.isParameterKnown() method
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
2021-06-24 18:05:14 +02:00
Thiemo Kreuz 30dc85f53b Better name for spec.getParameterName() method
This reflects much better what the method is for.

Bug: T285483
Change-Id: I7c90643421e32946fce4de813c7614b806b261f0
2021-06-24 18:02:52 +02:00
Thiemo Kreuz 3d02c1f364 Add new QUnit test for ve.dm.MWTemplateSpecModel
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
2021-06-24 17:59:38 +02:00