Re: [xwiki-devs] [xwiki-notifications] r12079 - in platform/core/trunk/xwiki-rendering/src/main/java/org/xwiki/rendering: macro transformation
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()); } }
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/
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.
On Wed, Aug 27, 2008 at 2:02 PM, Vincent Massol <[email protected]> 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.
On Oct 22, 2009, at 2:43 PM, Vincent Massol wrote:
On Wed, Aug 27, 2008 at 2:02 PM, Vincent Massol <[email protected]> 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.
participants (2)
-
Sergiu Dumitriu -
Vincent Massol