Bug 379783 - [WikiText] Expand MediaWiki template recursively
Summary: [WikiText] Expand MediaWiki template recursively
Status: RESOLVED FIXED
Alias: None
Product: z_Archived
Classification: Eclipse Foundation
Component: Mylyn (show other bugs)
Version: unspecified   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: 1.8.0   Edit
Assignee: Jeremie Bresson CLA
QA Contact: David Green CLA
URL:
Whiteboard:
Keywords: contributed
Depends on:
Blocks:
 
Reported: 2012-05-17 02:18 EDT by Jeremie Bresson CLA
Modified: 2012-07-09 13:46 EDT (History)
0 users

See Also:


Attachments
Patch for bug 379783 (1.96 KB, patch)
2012-05-17 02:20 EDT, Jeremie Bresson CLA
no flags Details | Diff
Loop detection screenshot (21.72 KB, image/png)
2012-05-17 02:29 EDT, Jeremie Bresson CLA
no flags Details
mylyn/context/zip (4.99 KB, application/octet-stream)
2012-05-17 17:36 EDT, David Green CLA
no flags Details
Patch for bug 379783 (version 2) (4.98 KB, patch)
2012-05-19 11:56 EDT, Jeremie Bresson CLA
no flags Details | Diff
mylyn/context/zip (5.69 KB, application/octet-stream)
2012-07-09 11:17 EDT, David Green CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremie Bresson CLA 2012-05-17 02:18:06 EDT
Build Identifier: 

MediaWiki handles Templates recursively: Templates can be included in other templates.
 
Example:
 
Template:foo
    _{{bar}}expanded_
 
Template:bar
    +exp+
 
 
"one {{foo}} two" will be resolved by mediaWiki as "one _+exp+expanded_ two".
The current Implementation of WikiText returns:
"one _{{bar}} expanded_ two"

Reproducible: Always
Comment 1 Jeremie Bresson CLA 2012-05-17 02:20:38 EDT
Created attachment 215749 [details]
Patch for bug 379783

Here a first patch for this bug.

The test testBasicTemplatesNoParametersRec() in TemplateProcessorTest illustrates the case described in the bug.

Nothing on loop detection (see next comment).
Comment 2 Jeremie Bresson CLA 2012-05-17 02:29:08 EDT
Created attachment 215750 [details]
Loop detection screenshot

MediaWiki handles a Loop Detection.

Here an example:

Template:mer
    "§test{{mer}}test§"

Template:rec1
    "+ rec1 {{rec2}} rec1 +"

Template:rec2
    "+ rec2 {{rec3}} rec2 +"

Template:rec3
    "+ rec3 {{rec1}} rec3 +"

In a page (see screenshot)
including {{mer}} is expanded as: "§testTemplate loop detected: Template:Mertest§" 

including {{rec1}} is exanded as: "+ rec1 + rec2 + rec3 Template loop detected: Template:Rec1 rec3 + rec2 + rec1 +"

* Do you think WikiText should also check for Loops?
* How should a loop be signalized? (In the output like MediaWiki, as an Exception ?)

I would be glad to correct my patch, if you give me some input on you vision.

Thanks.
Comment 3 David Green CLA 2012-05-17 17:36:46 EDT
Excellent patch, thank you.  I've pushed a review: https://git.eclipse.org/r/#change,6020

Yes, Mylyn WikiText should also check for loops/cycles in template processing.  

Generally Mylyn WikiText should attempt to process content without throwing exceptions since exceptions are not very useful to the user.  In most cases we try to emulate the behaviour of the reference implementation of a particular wiki markup (in this case MediaWiki).

I look forward to an updated patch!
Comment 4 David Green CLA 2012-05-17 17:36:47 EDT
Created attachment 215823 [details]
mylyn/context/zip
Comment 5 Jeremie Bresson CLA 2012-05-19 11:48:08 EDT
Here is the output from MediaWiki corresponding to the screenshot (attachment 215750 [details] [1]):

For the first example:
    §test<span class="error">Template loop detected: <a href="/mediawiki_root/index.php/Template:Mer" title="Template:Mer">Template:Mer</a></span>test§

For the second example:
    + rec1 + rec2 + rec3 <span class="error">Template loop detected: <a href="/mediawiki_root/index.php/Template:Rec1" title="Template:Rec1">Template:Rec1</a></span> rec3 + rec2 + rec1 +

* I am not so sure the '<a href="{url of template}" title="{nampe of template}">{nampe of template}</a>' makes sence for Wikitext (Wikitext is independent from the MediaWiki instance).
* Having a span class error suppose to have a good CSS (like red). Otherwise the user won't be aware of the conversion error due to the loop.

Here the CSS corresponding to error (default skin):
    .error {
        color: red;
        font-size: larger;
    }

[1] https://bugs.eclipse.org/bugs/attachment.cgi?id=215750
Comment 6 Jeremie Bresson CLA 2012-05-19 11:56:11 EDT
Created attachment 215902 [details]
Patch for bug 379783 (version 2)

Correction of the patch to detect loops

I implement the proposed examples as unit test (also in the patch).
I needed to change rec1, rec2, rec3 in rec_a, rec_b, rec_c because of Bug 380052.


The output is not exactly the same as MediaWiki (see previous comment):

First case:
    §test<span class=\"error\">Template loop detected:mer</span>test§

Second case:
    + rec_a + rec_b + rec_c <span class=\"error\">Template loop detected:rec_a</span> rec_c + rec_b + rec_a +
Comment 7 Jeremie Bresson CLA 2012-05-24 10:16:25 EDT
Uploaded patch set 2.
https://git.eclipse.org/r/6020
Comment 8 Jeremie Bresson CLA 2012-05-25 07:37:32 EDT
Uploaded patch set 3.
Comment 9 David Green CLA 2012-06-06 12:09:58 EDT
Jeremie, the patch looks great.  I'm planning on merging your patch as soon as we've cleared the "contribution quiet time":http://dev.eclipse.org/mhonarc/lists/mylyn-dev/msg01666.html for the Mylyn 3.8 release.

Assigning to you so that you get credit for the excellent contribution.
Comment 10 Jeremie Bresson CLA 2012-06-06 15:58:14 EDT
No problem to wait after RC3. It is not a problem for me to have a queue of changes in Gerrit.
Comment 11 Jeremie Bresson CLA 2012-06-11 01:02:08 EDT
Uploaded patch set 4.
https://git.eclipse.org/r/6020

* Rebase on head of master (to get the Hudson build the project)
* Change the commit message.
* Add a Message in Gerrit (vote 0) with the confirmation described in the Git Contribution process [1]

[1] http://wiki.eclipse.org/Development_Resources/Handling_Git_Contributions
Comment 12 David Green CLA 2012-07-09 11:17:08 EDT
Patch applied as commit 6d61fcabeee824df898541d6d18da9b63a755230.  Thanks for the contribution.
Comment 13 David Green CLA 2012-07-09 11:17:11 EDT
Created attachment 218443 [details]
mylyn/context/zip