Re: [xwiki-notifications] r12079 - in platform/core/trunk/xwiki-rendering/src/main/java/org/xwiki/rendering: macro transformation

[vmassol]

Hi Sergiu,

Are you sure about this below? What's your use case?

I find it strange to order by class name for an execution order.

Thanks
-Vincent

On Aug 27, 2008, at 1:59 AM, sdumitriu (SVN) wrote:

> Author: sdumitriu
> Date: 2008-08-27 01:59:21 +0200 (Wed, 27 Aug 2008)
> New Revision: 12079
>
> Modified:
>   platform/core/trunk/xwiki-rendering/src/main/java/org/xwiki/
> rendering/macro/AbstractMacro.java
>   platform/core/trunk/xwiki-rendering/src/main/java/org/xwiki/
> rendering/transformation/AbstractTransformation.java
> Log:
> XWIKI-2568: Continue implementing the new Rendering Architecture
> Improve the macro and transformation comparer so that component  
> implementations with the same priority are ordered by the classname.
>
>
> Modified: platform/core/trunk/xwiki-rendering/src/main/java/org/
> xwiki/rendering/macro/AbstractMacro.java
> ===================================================================
> --- platform/core/trunk/xwiki-rendering/src/main/java/org/xwiki/
> rendering/macro/AbstractMacro.java 2008-08-26 23:57:19 UTC (rev 12078)
> +++ platform/core/trunk/xwiki-rendering/src/main/java/org/xwiki/
> rendering/macro/AbstractMacro.java 2008-08-26 23:59:21 UTC (rev 12079)
> @@ -78,6 +78,9 @@
>      */
>     public int compareTo(Macro< ? > macro)
>     {
> -        return getPriority() - macro.getPriority();
> +        if (getPriority() != macro.getPriority()) {
> +            return getPriority() - macro.getPriority();
> +        }
> +        return  
> this
> .getClass
> ().getSimpleName().compareTo(macro.getClass().getSimpleName());
>     }
> }
>
> Modified: platform/core/trunk/xwiki-rendering/src/main/java/org/
> xwiki/rendering/transformation/AbstractTransformation.java
> ===================================================================
> --- platform/core/trunk/xwiki-rendering/src/main/java/org/xwiki/
> rendering/transformation/AbstractTransformation.java 2008-08-26  
> 23:57:19 UTC (rev 12078)
> +++ platform/core/trunk/xwiki-rendering/src/main/java/org/xwiki/
> rendering/transformation/AbstractTransformation.java 2008-08-26  
> 23:59:21 UTC (rev 12079)
> @@ -52,6 +52,9 @@
>      */
>     public int compareTo(Transformation transformation)
>     {
> -        return getPriority() - transformation.getPriority();
> +        if (getPriority() != transformation.getPriority()) {
> +            return getPriority() - transformation.getPriority();
> +        }
> +        return  
> this
> .getClass
> ().getSimpleName
> ().compareTo(transformation.getClass().getSimpleName());
>     }
> }

_______________________________________________
devs mailing list
[hidden email]
http://lists.xwiki.org/mailman/listinfo/devs

[Sergiu Dumitriu-2]

Vincent Massol wrote:
> Hi Sergiu,
>
> Are you sure about this below? What's your use case?
>
> I find it strange to order by class name for an execution order.
>

There is no use case, but I think that we should have a deterministic
rendering process. If we order only by the priority, then I don't know
in which order will the rendering happen, since most macros have
priority 1000. Ideally this shouldn't be a problem, but if there's a bug
somewhere, it will be harder to spot it if the execution order is always
changing. So, it's not something needed, but something safer. If you
don't have a counter use case, I'd rather keep it like this.

>
> On Aug 27, 2008, at 1:59 AM, sdumitriu (SVN) wrote:
>
>> Author: sdumitriu
>> Date: 2008-08-27 01:59:21 +0200 (Wed, 27 Aug 2008)
>> New Revision: 12079
>>
>> Modified:
>>   platform/core/trunk/xwiki-rendering/src/main/java/org/xwiki/
>> rendering/macro/AbstractMacro.java
>>   platform/core/trunk/xwiki-rendering/src/main/java/org/xwiki/
>> rendering/transformation/AbstractTransformation.java
>> Log:
>> XWIKI-2568: Continue implementing the new Rendering Architecture
>> Improve the macro and transformation comparer so that component  
>> implementations with the same priority are ordered by the classname.

--
Sergiu Dumitriu
http://purl.org/net/sergiu/
_______________________________________________
devs mailing list
[hidden email]
http://lists.xwiki.org/mailman/listinfo/devs

[vmassol]

On Aug 27, 2008, at 2:08 PM, Sergiu Dumitriu wrote:

> Vincent Massol wrote:
>> Hi Sergiu,
>>
>> Are you sure about this below? What's your use case?
>>
>> I find it strange to order by class name for an execution order.
>>
>
> There is no use case, but I think that we should have a deterministic
> rendering process. If we order only by the priority, then I don't know
> in which order will the rendering happen, since most macros have
> priority 1000. Ideally this shouldn't be a problem, but if there's a  
> bug
> somewhere, it will be harder to spot it if the execution order is  
> always
> changing. So, it's not something needed, but something safer. If you
> don't have a counter use case, I'd rather keep it like this.

The order is already deterministic AFAIK. It's the order in which the  
macros are defined in the components.xml.

Ok I see what you mean... what if the user also provides macro in  
another jar for example?

Let's keep it like this then for now.

Thanks
-Vincent

>> On Aug 27, 2008, at 1:59 AM, sdumitriu (SVN) wrote:
>>
>>> Author: sdumitriu
>>> Date: 2008-08-27 01:59:21 +0200 (Wed, 27 Aug 2008)
>>> New Revision: 12079
>>>
>>> Modified:
>>>  platform/core/trunk/xwiki-rendering/src/main/java/org/xwiki/
>>> rendering/macro/AbstractMacro.java
>>>  platform/core/trunk/xwiki-rendering/src/main/java/org/xwiki/
>>> rendering/transformation/AbstractTransformation.java
>>> Log:
>>> XWIKI-2568: Continue implementing the new Rendering Architecture
>>> Improve the macro and transformation comparer so that component
>>> implementations with the same priority are ordered by the classname.

_______________________________________________
devs mailing list
[hidden email]
http://lists.xwiki.org/mailman/listinfo/devs

[vmassol]

On Wed, Aug 27, 2008 at 2:02 PM, Vincent Massol <[hidden email]> wrote:

>
> On Aug 27, 2008, at 2:08 PM, Sergiu Dumitriu wrote:
>
>> Vincent Massol wrote:
>>> Hi Sergiu,
>>>
>>> Are you sure about this below? What's your use case?
>>>
>>> I find it strange to order by class name for an execution order.
>>>
>>
>> There is no use case, but I think that we should have a deterministic
>> rendering process. If we order only by the priority, then I don't know
>> in which order will the rendering happen, since most macros have
>> priority 1000. Ideally this shouldn't be a problem, but if there's a
>> bug
>> somewhere, it will be harder to spot it if the execution order is
>> always
>> changing. So, it's not something needed, but something safer. If you
>> don't have a counter use case, I'd rather keep it like this.
>
> The order is already deterministic AFAIK. It's the order in which the
> macros are defined in the components.xml.
>
> Ok I see what you mean... what if the user also provides macro in
> another jar for example?
>
> Let's keep it like this then for now.

This is actually wrong. The order should only be on the priority and
the macro name shouldn't be mixed.

What should happen IMO is that macro order should be that of the page,
i.e. if macro1 is before macro2 and they both have the same priority
then macro1 executes before macro2.

I'll look at fixing this since the current algo is wrong.

Thanks
-Vincent

> Thanks
> -Vincent
>
>>> On Aug 27, 2008, at 1:59 AM, sdumitriu (SVN) wrote:
>>>
>>>> Author: sdumitriu
>>>> Date: 2008-08-27 01:59:21 +0200 (Wed, 27 Aug 2008)
>>>> New Revision: 12079
>>>>
>>>> Modified:
>>>>  platform/core/trunk/xwiki-rendering/src/main/java/org/xwiki/
>>>> rendering/macro/AbstractMacro.java
>>>>  platform/core/trunk/xwiki-rendering/src/main/java/org/xwiki/
>>>> rendering/transformation/AbstractTransformation.java
>>>> Log:
>>>> XWIKI-2568: Continue implementing the new Rendering Architecture
>>>> Improve the macro and transformation comparer so that component
>>>> implementations with the same priority are ordered by the classname.

_______________________________________________
devs mailing list
[hidden email]
http://lists.xwiki.org/mailman/listinfo/devs

[vmassol]

On Oct 22, 2009, at 2:43 PM, Vincent Massol wrote:

> On Wed, Aug 27, 2008 at 2:02 PM, Vincent Massol <[hidden email]>  
> wrote:
>>
>> On Aug 27, 2008, at 2:08 PM, Sergiu Dumitriu wrote:
>>
>>> Vincent Massol wrote:
>>>> Hi Sergiu,
>>>>
>>>> Are you sure about this below? What's your use case?
>>>>
>>>> I find it strange to order by class name for an execution order.
>>>>
>>>
>>> There is no use case, but I think that we should have a  
>>> deterministic
>>> rendering process. If we order only by the priority, then I don't  
>>> know
>>> in which order will the rendering happen, since most macros have
>>> priority 1000. Ideally this shouldn't be a problem, but if there's a
>>> bug
>>> somewhere, it will be harder to spot it if the execution order is
>>> always
>>> changing. So, it's not something needed, but something safer. If you
>>> don't have a counter use case, I'd rather keep it like this.
>>
>> The order is already deterministic AFAIK. It's the order in which the
>> macros are defined in the components.xml.
>>
>> Ok I see what you mean... what if the user also provides macro in
>> another jar for example?
>>
>> Let's keep it like this then for now.
>
> This is actually wrong. The order should only be on the priority and
> the macro name shouldn't be mixed.
>
> What should happen IMO is that macro order should be that of the page,
> i.e. if macro1 is before macro2 and they both have the same priority
> then macro1 executes before macro2.
>
> I'll look at fixing this since the current algo is wrong.

Even worse Sergiu. Your change as actually broken macro rendering...

Example:

{{ruby}}
def var1
{{/ruby}}

{{groovy}}
use var1
{{/groovy}}

will fail since the groovy macro will be executed before the ruby macro.

I'm fixing it.

-Vincent

> Thanks
> -Vincent
>
>> Thanks
>> -Vincent
>>
>>>> On Aug 27, 2008, at 1:59 AM, sdumitriu (SVN) wrote:
>>>>
>>>>> Author: sdumitriu
>>>>> Date: 2008-08-27 01:59:21 +0200 (Wed, 27 Aug 2008)
>>>>> New Revision: 12079
>>>>>
>>>>> Modified:
>>>>>  platform/core/trunk/xwiki-rendering/src/main/java/org/xwiki/
>>>>> rendering/macro/AbstractMacro.java
>>>>>  platform/core/trunk/xwiki-rendering/src/main/java/org/xwiki/
>>>>> rendering/transformation/AbstractTransformation.java
>>>>> Log:
>>>>> XWIKI-2568: Continue implementing the new Rendering Architecture
>>>>> Improve the macro and transformation comparer so that component
>>>>> implementations with the same priority are ordered by the  
>>>>> classname.

_______________________________________________
devs mailing list
[hidden email]
http://lists.xwiki.org/mailman/listinfo/devs