On 4 Jul 2018, at 12:07, Thomas Mortagne <thomas.mortagne@xwiki.com> wrote:

Here are more details on the actual use case we need to support:

In include/Display macro either you set:

* "reference" and "type" (which default to DOCUMENT) * or you set “page"

Globally I think we need to add 3 concepts to macro parameter descriptor: 1) The concept of “deprecated” parameter. For example for “document” in the include macro. 2) The concept of aliases or groups, i.e the ability to list parameters that are mutually exclusive. Example: reference + type vs page for display/include macros. This would mean that in the Macro Dialog UI if you select one of those the other gets unselected/cleared out (you cannot have mutually exclusive params have values). 3) The concept of Advanced parameters. For example, we should put reference + type as advanced parameters so that they are not shown to the user by default (and so that the page parameter is more highlighted). Users would need to click on Advanced to see advanced parameters. I think we’re doing something automatic today (I don’t remember the details) to try to hide some parameters but we should probably review this. WDYT? Thanks -Vincent

On Wed, Jul 4, 2018 at 10:57 AM, Marius Dumitru Florea <mariusdumitru.florea@xwiki.com> wrote:

...

For groups of parameters we could use tabs:

----------8<---------- Param 1 [input]

Param 2 | Param 3 | Param 4 <--- a group of 3 parameters displayed using tabs (we take into account only the value of the parameter from the active tab) [input for param 2]

Param 5 [input] ---------->8----------

But if you want to support subgroups of parameters also then it becomes more complicated.

Thanks, Marius

On Mon, Jul 2, 2018 at 11:52 AM, Thomas Mortagne <thomas.mortagne@xwiki.com> wrote:

...

Hi xwikiers,

In the contact of bringing new Page concept (OK 7.4 is starting to get old) to the API and macros too we decided (1) to introduce a "page" shortcut property (even if we keep the reference/type for other types).

While it's nicer for wiki syntax, one issue is that on WYSIWYG macros UI side, which display all properties, it means ending up with conflicting parameters that needs to be displayed as such.

I don't really have much clue on how best to display this so I'm searching for ideas :)

Then I will add in the macro descriptor what's required for whatever UI we want to build (group and sub groups of properties, etc.).

1: http://design.xwiki.org/xwiki/bin/view/Proposal/ DeprecatingSpaceAndSpaceReference#HMacros

Thanks, -- Thomas Mortagnes

-- Thomas Mortagne

On Wed, Jul 18, 2018 at 5:00 PM Vincent Massol <vincent@massol.net> wrote:

...

On 5 Jul 2018, at 12:06, Vincent Massol <vincent@massol.net> wrote:

...

On 4 Jul 2018, at 12:07, Thomas Mortagne <thomas.mortagne@xwiki.com> wrote:

Here are more details on the actual use case we need to support:

In include/Display macro either you set:

* "reference" and "type" (which default to DOCUMENT) * or you set “page"

Globally I think we need to add 3 concepts to macro parameter descriptor:

1) The concept of “deprecated” parameter. For example for “document” in the include macro. 2) The concept of aliases or groups, i.e the ability to list parameters that are mutually exclusive. Example: reference + type vs page for display/include macros. This would mean that in the Macro Dialog UI if you select one of those the other gets unselected/cleared out (you cannot have mutually exclusive params have values). 3) The concept of Advanced parameters. For example, we should put reference + type as advanced parameters so that they are not shown to the user by default (and so that the page parameter is more highlighted). Users would need to click on Advanced to see advanced parameters. I think we’re doing something automatic today (I don’t remember the details) to try to hide some parameters but we should probably review this.

WDYT?

Ping!

Do we agree about this? If we do we can then create jira issue about it and take it for implementation.

+1, I can create the jira issue if it's ok.

Thanks -Vincent

...

Thanks -Vincent

...

On Wed, Jul 4, 2018 at 10:57 AM, Marius Dumitru Florea <mariusdumitru.florea@xwiki.com> wrote:

...

For groups of parameters we could use tabs:

----------8<---------- Param 1 [input]

Param 2 | Param 3 | Param 4 <--- a group of 3 parameters displayed using tabs (we take into account only the value of the parameter from the active tab) [input for param 2]

Param 5 [input] ---------->8----------

But if you want to support subgroups of parameters also then it becomes more complicated.

Thanks, Marius

On Mon, Jul 2, 2018 at 11:52 AM, Thomas Mortagne <thomas.mortagne@xwiki.com> wrote:

...

Hi xwikiers,

In the contact of bringing new Page concept (OK 7.4 is starting to get old) to the API and macros too we decided (1) to introduce a "page" shortcut property (even if we keep the reference/type for other types).

While it's nicer for wiki syntax, one issue is that on WYSIWYG macros UI side, which display all properties, it means ending up with conflicting parameters that needs to be displayed as such.

I don't really have much clue on how best to display this so I'm searching for ideas :)

Then I will add in the macro descriptor what's required for whatever UI we want to build (group and sub groups of properties, etc.).

1: http://design.xwiki.org/xwiki/bin/view/Proposal/ DeprecatingSpaceAndSpaceReference#HMacros

Thanks, -- Thomas Mortagnes

-- Thomas Mortagne

On Wed, Sep 19, 2018 at 4:31 PM Vincent Massol <vincent@massol.net> wrote:

...

On 19 Sep 2018, at 14:47, Adel Atallah <adel.atallah@xwiki.com> wrote:

On Wed, Jul 18, 2018 at 5:00 PM Vincent Massol <vincent@massol.net> wrote:

...

...

On 5 Jul 2018, at 12:06, Vincent Massol <vincent@massol.net> wrote:

...

On 4 Jul 2018, at 12:07, Thomas Mortagne <thomas.mortagne@xwiki.com>

wrote:

...

...

...

Here are more details on the actual use case we need to support:

In include/Display macro either you set:

* "reference" and "type" (which default to DOCUMENT) * or you set “page"

Globally I think we need to add 3 concepts to macro parameter descriptor:

1) The concept of “deprecated” parameter. For example for “document” in the include macro. 2) The concept of aliases or groups, i.e the ability to list parameters that are mutually exclusive. Example: reference + type vs page for display/include macros. This would mean that in the Macro Dialog UI if you select one of those the other gets unselected/cleared out (you cannot have mutually exclusive params have values). 3) The concept of Advanced parameters. For example, we should put reference + type as advanced parameters so that they are not shown to the user by default (and so that the page parameter is more highlighted). Users would need to click on Advanced to see advanced parameters. I think we’re doing something automatic today (I don’t remember the details) to try to hide some parameters but we should probably review this.

WDYT?

Ping!

Do we agree about this? If we do we can then create jira issue about it and take it for implementation.

+1, I can create the jira issue if it's ok.

Please do :)

@Marius: Ok for you?

Yes.

thanks -Vincent

[snip]

Note: this should be done at xwiki-commons-properties level. Macro parameters are just one use case. On Wed, Nov 7, 2018 at 4:34 PM Adel Atallah <adel.atallah@xwiki.com> wrote:

Hello everyone,

So what we thought about with Vincent for implementing the "concept of aliases or groups" would be to actually have two new annotations that we would use on macro properties. The first one is a "Group" annotation which is meant to indicate that some properties are part of the same group, obviously. The second is an "Alternative" annotation which is meant to indicate that only one property / group of properties can be used (among the ones that are part of the alternative). Here is an example: We want for the Include macro to be able to specify either: the "reference" and "type" parameters or the "page" parameter For that, we will change the IncludeMacroParameters java class like this:

@Alternative("reference") @Group("entityReference") public void setReference(String reference)

@Alternative("reference") @Group("entityReference") public void setType(EntityType type)

@Alternative("reference") public void setPage(String page)

In the WYSIWYG side, we will only be able to specify either the "reference" and the "type" or the "page" parameter.

WDYT?

Thanks, Adel

On Tue, Sep 25, 2018 at 11:51 AM Marius Dumitru Florea <mariusdumitru.florea@xwiki.com> wrote:

...

On Wed, Sep 19, 2018 at 4:31 PM Vincent Massol <vincent@massol.net> wrote:

...

...

On 19 Sep 2018, at 14:47, Adel Atallah <adel.atallah@xwiki.com> wrote:

On Wed, Jul 18, 2018 at 5:00 PM Vincent Massol <vincent@massol.net> wrote:

...

...

On 5 Jul 2018, at 12:06, Vincent Massol <vincent@massol.net> wrote:

> On 4 Jul 2018, at 12:07, Thomas Mortagne <thomas.mortagne@xwiki.com>

wrote:

...

...

> > Here are more details on the actual use case we need to support: > > In include/Display macro either you set: > > * "reference" and "type" (which default to DOCUMENT) > * or you set “page"

Globally I think we need to add 3 concepts to macro parameter descriptor:

1) The concept of “deprecated” parameter. For example for “document” in the include macro. 2) The concept of aliases or groups, i.e the ability to list parameters that are mutually exclusive. Example: reference + type vs page for display/include macros. This would mean that in the Macro Dialog UI if you select one of those the other gets unselected/cleared out (you cannot have mutually exclusive params have values). 3) The concept of Advanced parameters. For example, we should put reference + type as advanced parameters so that they are not shown to the user by default (and so that the page parameter is more highlighted). Users would need to click on Advanced to see advanced parameters. I think we’re doing something automatic today (I don’t remember the details) to try to hide some parameters but we should probably review this.

WDYT?

Ping!

Do we agree about this? If we do we can then create jira issue about it and take it for implementation.

+1, I can create the jira issue if it's ok.

Please do :)

...

@Marius: Ok for you?

Yes.

...

thanks -Vincent

[snip]

-- Thomas Mortagne

Hi,

On 9 Nov 2018, at 11:20, Marius Dumitru Florea <mariusdumitru.florea@xwiki.com> wrote:

On Wed, Nov 7, 2018 at 5:34 PM Adel Atallah <adel.atallah@xwiki.com> wrote:

...

Hello everyone,

So what we thought about with Vincent for implementing the "concept of aliases or groups" would be to actually have two new annotations that we would use on macro properties. The first one is a "Group" annotation which is meant to indicate that some properties are part of the same group, obviously. The second is an "Alternative" annotation which is meant to indicate that only one property / group of properties can be used (among the ones that are part of the alternative). Here is an example: We want for the Include macro to be able to specify either: the "reference" and "type" parameters or the "page" parameter For that, we will change the IncludeMacroParameters java class like this:

@Alternative("reference") @Group("entityReference") public void setReference(String reference)

@Alternative("reference") @Group("entityReference") public void setType(EntityType type)

@Alternative("reference") public void setPage(String page)

In the WYSIWYG side, we will only be able to specify either the "reference" and the "type" or the "page" parameter.

I think it would make more sense, at least in this case, to have the alternative as an attribute of the group, because semantically the "entityReference" group is an alternative to the page parameter. You can't say that the type parameter alone is an alternative to the page parameter.

The @Group annotation is clear. No doubt about it. I'm not sure about the @Alternative annotation. I'm thinking that the "alternative" is also a group, where only one item from the group can be used, which could be expressed with an attribute of the @Group annotation.

For me the concepts of Groups and Alternatives are separate. For example you could imagine defining a group of properties so that the WYSIWYG would display them together, one under another or with some box border around them. Alternatives don’t need to be on groups. You can have alternatives on individual properties or alternatives between 1 property and a group or alternatives between one group and another. Thanks -Vincent [snip]

On Fri, Nov 9, 2018 at 11:57 AM Marius Dumitru Florea <mariusdumitru.florea@xwiki.com> wrote:

On Fri, Nov 9, 2018 at 12:29 PM Vincent Massol <vincent@massol.net> wrote:

...

Hi,

...

On 9 Nov 2018, at 11:20, Marius Dumitru Florea < mariusdumitru.florea@xwiki.com> wrote:

On Wed, Nov 7, 2018 at 5:34 PM Adel Atallah <adel.atallah@xwiki.com> wrote:

...

Hello everyone,

So what we thought about with Vincent for implementing the "concept of aliases or groups" would be to actually have two new annotations that we would use on macro properties. The first one is a "Group" annotation which is meant to indicate that some properties are part of the same group, obviously. The second is an "Alternative" annotation which is meant to indicate that only one property / group of properties can be used (among the ones that are part of the alternative). Here is an example: We want for the Include macro to be able to specify either: the "reference" and "type" parameters or the "page" parameter For that, we will change the IncludeMacroParameters java class like this:

@Alternative("reference") @Group("entityReference") public void setReference(String reference)

@Alternative("reference") @Group("entityReference") public void setType(EntityType type)

@Alternative("reference") public void setPage(String page)

In the WYSIWYG side, we will only be able to specify either the "reference" and the "type" or the "page" parameter.

I think it would make more sense, at least in this case, to have the alternative as an attribute of the group, because semantically the "entityReference" group is an alternative to the page parameter. You can't say that the type parameter alone is an alternative to the page parameter.

The @Group annotation is clear. No doubt about it. I'm not sure about the @Alternative annotation. I'm thinking that the "alternative" is also a group, where only one item from the group can be used, which could be expressed with an attribute of the @Group annotation.

For me the concepts of Groups and Alternatives are separate. For example you could imagine defining a group of properties so that the WYSIWYG would display them together, one under another or with some box border around them.

...

Alternatives don’t need to be on groups.

I did not say the alternatives must be **on** groups. I said the alternatives **are** groups. When you have two alternative parameters then those 2 parameters are in an alternative **group**. That's what I said. Whether the groups is used just for display or for enforcing exclusive usage is something that could be expressed using annotation attributes.

I'm not sure to understand what you mean. Could you give us an example?

...

You can have alternatives on individual properties or alternatives between 1 property and a group or alternatives between one group and another.

Thanks -Vincent

[snip]

Try to view the macro parameters listed as a boolean expression. For the include macro we would have: (page XOR (reference AND type) XOR document) OR section OR context * the parentheses define the parameter groups * the boolean operators specify the relation between the members of a group We then need to express this using Java annotations. In any case, this is a **tree** structure (not a flat structure). On Fri, Nov 9, 2018 at 12:30 PM Adel Atallah <adel.atallah@xwiki.com> wrote:

On Fri, Nov 9, 2018 at 11:20 AM Marius Dumitru Florea <mariusdumitru.florea@xwiki.com> wrote:

...

On Wed, Nov 7, 2018 at 5:34 PM Adel Atallah <adel.atallah@xwiki.com>

wrote:

...

...

Hello everyone,

So what we thought about with Vincent for implementing the "concept of aliases or groups" would be to actually have two new annotations that we would use on macro properties. The first one is a "Group" annotation which is meant to indicate that some properties are part of the same group, obviously. The second is an "Alternative" annotation which is meant to indicate that only one property / group of properties can be used (among the ones that are part of the alternative). Here is an example: We want for the Include macro to be able to specify either: the "reference" and "type" parameters or the "page" parameter For that, we will change the IncludeMacroParameters java class like

this:

...

...

@Alternative("reference") @Group("entityReference") public void setReference(String reference)

@Alternative("reference") @Group("entityReference") public void setType(EntityType type)

@Alternative("reference") public void setPage(String page)

In the WYSIWYG side, we will only be able to specify either the "reference" and the "type" or the "page" parameter.

I think it would make more sense, at least in this case, to have the alternative as an attribute of the group, because semantically the "entityReference" group is an alternative to the page parameter. You can't say that the type parameter alone is an alternative to the page parameter.

The @Group annotation is clear. No doubt about it. I'm not sure about the @Alternative annotation. I'm thinking that the "alternative" is also a group, where only one item from the group can be used, which could be expressed with an attribute of the @Group annotation.

Thanks for the suggestion, but how can it be used? If I retake my previous example, will it be:

@Group(name = "entityReference", alternative = "reference") public void setReference(String reference)

@Group(name = "entityReference", alternative = "reference") public void setType(EntityType type)

@Group(name = "page", alternative = "reference") public void setPage(String page)

?

...

...

WDYT?

Thanks, Adel

On Tue, Sep 25, 2018 at 11:51 AM Marius Dumitru Florea <mariusdumitru.florea@xwiki.com> wrote:

...

On Wed, Sep 19, 2018 at 4:31 PM Vincent Massol <vincent@massol.net>

wrote:

...

...

...

On 19 Sep 2018, at 14:47, Adel Atallah <adel.atallah@xwiki.com>

wrote:

...

...

...

On Wed, Jul 18, 2018 at 5:00 PM Vincent Massol <

vincent@massol.net>

...

...

...

...

wrote:

...

> > > >> On 5 Jul 2018, at 12:06, Vincent Massol <vincent@massol.net> wrote: >> >> >> >>> On 4 Jul 2018, at 12:07, Thomas Mortagne < thomas.mortagne@xwiki.com> wrote: >>> >>> Here are more details on the actual use case we need to support: >>> >>> In include/Display macro either you set: >>> >>> * "reference" and "type" (which default to DOCUMENT) >>> * or you set “page" >> >> Globally I think we need to add 3 concepts to macro parameter descriptor: >> >> 1) The concept of “deprecated” parameter. For example for “document” in the include macro. >> 2) The concept of aliases or groups, i.e the ability to list parameters that are mutually exclusive. Example: reference + type vs page for display/include macros. This would mean that in the Macro Dialog UI if you select one of those the other gets unselected/cleared out (you cannot have mutually exclusive params have values). >> 3) The concept of Advanced parameters. For example, we should put reference + type as advanced parameters so that they are not shown to the user by default (and so that the page parameter is more highlighted). Users would need to click on Advanced to see advanced parameters. I think we’re doing something automatic today (I don’t remember the details) to try to hide some parameters but we should probably review this. >> >> WDYT? > > Ping! > > Do we agree about this? If we do we can then create jira issue about it and take it for implementation.

+1, I can create the jira issue if it's ok.

Please do :)

...

@Marius: Ok for you?

Yes.

...

thanks -Vincent

[snip]

On Fri, Nov 9, 2018 at 4:58 PM Adel Atallah <adel.atallah@xwiki.com> wrote:

On Fri, Nov 9, 2018 at 12:08 PM Marius Dumitru Florea <mariusdumitru.florea@xwiki.com> wrote:

...

Try to view the macro parameters listed as a boolean expression. For the include macro we would have:

(page XOR (reference AND type) XOR document) OR section OR context

Wouldn't it be: (page XOR (reference AND type) XOR document) *AND* section *AND* context ?

No. AND means (logically) both sides are required. You can specify the section parameter without specifying the context parameter. So it's definitely a *logical / boolean* OR between them. I put AND between reference and type just to show that there might be cases when two (or more) parameters must be specified together (i.e. either you specify both or you specify none of them). In our case the type parameter has a default value so you can specify only the reference. This means it's actually OR between reference and type in the case of the include macro.

I just feel like you can just have 2 types of "operators": one to exclude (XOR) and the other to include (AND) parameters.

I still think there can be 3 operators (if we want to cover everything). As I said, you may want to express the fact that a parameter is mandatory only if some other parameter is specified. We can't express this ATM so I guess we can work only with XOR and OR.

...

* the parentheses define the parameter groups * the boolean operators specify the relation between the members of a

group

...

We then need to express this using Java annotations. In any case, this

is a

...

**tree** structure (not a flat structure).

What you describe could probably be done with what we proposed but maybe we can have something more explicit:

public void setReference(String reference)

@Depends("reference") public void setType(EntityType type)

@Conflict("reference") public void setPage(String page)

Could that work? We would need to write all the logic behind to build the tree: 1. All parameters are joined by an 'AND' by default => page AND reference AND type 2. Page and Reference are conflicting => (page XOR reference) AND type 3. Type depends on Reference => (page XOR (reference AND type))

WDYT?

I'm not sure how @Depends and @Conflict defines the tree structure in general. I was rather thinking of something like: @GroupPath("target[XOR]") page @GroupPath("target[XOR]/entityReference") reference @GroupPath("target[XOR]/entityReference") type @GroupPath("target[XOR]") document If we want subgroups then we need to specify a path. The complex part is to specify the "operator" that should be used within a subgroup.

...

On Fri, Nov 9, 2018 at 12:30 PM Adel Atallah <adel.atallah@xwiki.com>

wrote:

...

...

On Fri, Nov 9, 2018 at 11:20 AM Marius Dumitru Florea <mariusdumitru.florea@xwiki.com> wrote:

...

On Wed, Nov 7, 2018 at 5:34 PM Adel Atallah <adel.atallah@xwiki.com>

wrote:

...

...

Hello everyone,

So what we thought about with Vincent for implementing the

"concept of

...

...

...

...

aliases or groups" would be to actually have two new annotations that we would use on macro properties. The first one is a "Group" annotation which is meant to indicate that some properties are part of the same group, obviously. The second is an "Alternative" annotation which is meant to indicate that only one property / group of properties can be used (among the ones that are part of the alternative). Here is an example: We want for the Include macro to be able to specify either: the "reference" and "type" parameters or the "page" parameter For that, we will change the IncludeMacroParameters java class like this:

@Alternative("reference") @Group("entityReference") public void setReference(String reference)

@Alternative("reference") @Group("entityReference") public void setType(EntityType type)

@Alternative("reference") public void setPage(String page)

In the WYSIWYG side, we will only be able to specify either the "reference" and the "type" or the "page" parameter.

I think it would make more sense, at least in this case, to have the alternative as an attribute of the group, because semantically the "entityReference" group is an alternative to the page parameter. You can't say that the type parameter alone is an alternative to the page parameter.

The @Group annotation is clear. No doubt about it. I'm not sure about the @Alternative annotation. I'm thinking that the "alternative" is also a group, where only one item from the group can be used, which could be expressed with an attribute of the @Group annotation.

Thanks for the suggestion, but how can it be used? If I retake my previous example, will it be:

@Group(name = "entityReference", alternative = "reference") public void setReference(String reference)

@Group(name = "entityReference", alternative = "reference") public void setType(EntityType type)

@Group(name = "page", alternative = "reference") public void setPage(String page)

?

...

...

WDYT?

Thanks, Adel

On Tue, Sep 25, 2018 at 11:51 AM Marius Dumitru Florea <mariusdumitru.florea@xwiki.com> wrote:

...

On Wed, Sep 19, 2018 at 4:31 PM Vincent Massol <

vincent@massol.net>

...

...

wrote:

...

> > > > On 19 Sep 2018, at 14:47, Adel Atallah <

adel.atallah@xwiki.com> wrote:

...

> > > > On Wed, Jul 18, 2018 at 5:00 PM Vincent Massol < vincent@massol.net> > wrote: > >> > >> > >> > >>> On 5 Jul 2018, at 12:06, Vincent Massol < vincent@massol.net> wrote: > >>> > >>> > >>> > >>>> On 4 Jul 2018, at 12:07, Thomas Mortagne < thomas.mortagne@xwiki.com> > wrote: > >>>> > >>>> Here are more details on the actual use case we need to support: > >>>> > >>>> In include/Display macro either you set: > >>>> > >>>> * "reference" and "type" (which default to DOCUMENT) > >>>> * or you set “page" > >>> > >>> Globally I think we need to add 3 concepts to macro parameter > descriptor: > >>> > >>> 1) The concept of “deprecated” parameter. For example for “document” > in the include macro. > >>> 2) The concept of aliases or groups, i.e the ability to list > parameters that are mutually exclusive. Example: reference + type vs page > for display/include macros. This would mean that in the Macro Dialog UI if > you select one of those the other gets unselected/cleared out (you cannot > have mutually exclusive params have values). > >>> 3) The concept of Advanced parameters. For example, we should put > reference + type as advanced parameters so that they are not shown to the > user by default (and so that the page parameter is more highlighted). Users > would need to click on Advanced to see advanced parameters. I think we’re > doing something automatic today (I don’t remember the details) to try to > hide some parameters but we should probably review this. > >>> > >>> WDYT? > >> > >> Ping! > >> > >> Do we agree about this? If we do we can then create jira issue about it > and take it for implementation. > > > > +1, I can create the jira issue if it's ok. > > Please do :) > >

> @Marius: Ok for you? >

Yes.

> > thanks > -Vincent > > [snip] > > >

Hello, I'd like to briefly summarize the situation so that we can make some progress. What we have: * We define "parameters" in a macro by creating a Java Bean, which provides all the getters and setters of the parameters we want. * We can use annotations on these getters/setters to define some behavior or metadata for these parameters (description, mandatory, deprecated...) What we want: * Being able to handle conflicting parameters. For instance when we deprecate a parameter and add a new one to replace it, we should be able to either use the deprecated parameter or the new one but not both. * We also want to group parameters that are related to each other. What we proposed: * Use annotations on the parameters to express the conflict. * Marius proposed to see the problem as a boolean expression such as: (page XOR (reference AND type) XOR document) OR section OR context. This would translate as: the user can use the 'section' and/or 'context' parameters (if they want), can use only one of these parameters: 'page', ('reference' and 'type') or 'document', where 'reference' and 'type' depend on each other and you can't use one without the other. * You can see on previous e-mails the kind of annotations we proposed to solve the issue. Thanks, Adel

I thought about something like this but I discarded it as I find this complicated for something that should be relatively simple. I’d prefer to have some simple annotations if possible. In other words, if feels a bit of over-engineering for the need. Now I have to admit that I stopped following this thread after the original proposal so maybe I’m just completely off :) Thanks -Vincent

On 14 Nov 2018, at 15:51, Marius Dumitru Florea <mariusdumitru.florea@xwiki.com> wrote:

WDYT about:

-----8<----- IncludeMacroParameters ---------- @Group("target") page

@Group("target/entityReference") reference

@Group("target/entityReference") type

@Group("target") document

section

context ----->8---------------

That is: specify *only* the group hierarchy in the macro parameter descriptor. This would produce the following hierarchy:

* <target> ** page ** <entityReference> *** reference *** type ** document * section * context

Next, for the cases where we want to customize the behavior of a group, we introduce a component role ParameterGroup. For instance, for the "target" parameter group of the Include Macro we would create

@Named("include/target") public class TargetParameterGroup implements ParameterGroup {}

To specify that the members of a parameter group are exclusive we can either use a method in the ParameterGroup interface (e.g. isExclusive()) or use an annotation on the implementation TargetParameterGroup.

Thanks, Marius

On Tue, Nov 13, 2018 at 12:03 PM Adel Atallah <adel.atallah@xwiki.com> wrote:

...

Hello,

I'd like to briefly summarize the situation so that we can make some progress.

What we have: * We define "parameters" in a macro by creating a Java Bean, which provides all the getters and setters of the parameters we want. * We can use annotations on these getters/setters to define some behavior or metadata for these parameters (description, mandatory, deprecated...)

What we want: * Being able to handle conflicting parameters. For instance when we deprecate a parameter and add a new one to replace it, we should be able to either use the deprecated parameter or the new one but not both. * We also want to group parameters that are related to each other.

What we proposed: * Use annotations on the parameters to express the conflict. * Marius proposed to see the problem as a boolean expression such as: (page XOR (reference AND type) XOR document) OR section OR context. This would translate as: the user can use the 'section' and/or 'context' parameters (if they want), can use only one of these parameters: 'page', ('reference' and 'type') or 'document', where 'reference' and 'type' depend on each other and you can't use one without the other. * You can see on previous e-mails the kind of annotations we proposed to solve the issue.

Thanks, Adel

On 15 Nov 2018, at 06:29, Marius Dumitru Florea <mariusdumitru.florea@xwiki.com> wrote:

On Wed, Nov 14, 2018 at 5:12 PM Vincent Massol <vincent@massol.net> wrote:

...

I thought about something like this but I discarded it as I find this complicated for something that should be relatively simple.

I don't think it's that complicated because:

* Conflicting parameters should be an exception, not the rule. What other macros, besides include / display, need this? * If you just want to group macro parameters for display then you only need to use the @Group annotation. You don't need to implement a ParameterGroup. The ParameterGroup is needed only for conflicting parameters (ATM).

Sure but it’s still 10x more complicated than just having everything in one place in the parameters class with annotations as was suggested initially. Thanks -Vincent

Thanks, Marius

...

I’d prefer to have some simple annotations if possible. In other words, if feels a bit of over-engineering for the need. Now I have to admit that I stopped following this thread after the original proposal so maybe I’m just completely off :)

Thanks -Vincent

...

On 14 Nov 2018, at 15:51, Marius Dumitru Florea < mariusdumitru.florea@xwiki.com> wrote:

WDYT about:

-----8<----- IncludeMacroParameters ---------- @Group("target") page

@Group("target/entityReference") reference

@Group("target/entityReference") type

@Group("target") document

section

context ----->8---------------

That is: specify *only* the group hierarchy in the macro parameter descriptor. This would produce the following hierarchy:

* <target> ** page ** <entityReference> *** reference *** type ** document * section * context

Next, for the cases where we want to customize the behavior of a group, we introduce a component role ParameterGroup. For instance, for the "target" parameter group of the Include Macro we would create

@Named("include/target") public class TargetParameterGroup implements ParameterGroup {}

To specify that the members of a parameter group are exclusive we can either use a method in the ParameterGroup interface (e.g. isExclusive()) or use an annotation on the implementation TargetParameterGroup.

Thanks, Marius

On Tue, Nov 13, 2018 at 12:03 PM Adel Atallah <adel.atallah@xwiki.com> wrote:

...

Hello,

I'd like to briefly summarize the situation so that we can make some progress.

What we have: * We define "parameters" in a macro by creating a Java Bean, which provides all the getters and setters of the parameters we want. * We can use annotations on these getters/setters to define some behavior or metadata for these parameters (description, mandatory, deprecated...)

What we want: * Being able to handle conflicting parameters. For instance when we deprecate a parameter and add a new one to replace it, we should be able to either use the deprecated parameter or the new one but not both. * We also want to group parameters that are related to each other.

What we proposed: * Use annotations on the parameters to express the conflict. * Marius proposed to see the problem as a boolean expression such as: (page XOR (reference AND type) XOR document) OR section OR context. This would translate as: the user can use the 'section' and/or 'context' parameters (if they want), can use only one of these parameters: 'page', ('reference' and 'type') or 'document', where 'reference' and 'type' depend on each other and you can't use one without the other. * You can see on previous e-mails the kind of annotations we proposed to solve the issue.

Thanks, Adel

I'm also really not a fan of having to implement a component just to indicate that two groups of properties are conflicting. +1 for making @Group support a hierarchy, that's indeed nice. For for conflicting we need a dedicated annotation IMO. So starting from your previous example I would expect something like:

...

...

...

...

...

>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>

@PropertyGroup("target") @PropertyFeature("reference") page @PropertyGroup({"target", "entityReference"}) @PropertyFeature("reference") reference @PropertyGroup({"target", "entityReference"}) type @PropertyGroup("target") @PropertyFeature("reference") document <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< or

...

...

...

...

...

>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>

@PropertyGroup("target", features = "reference") page @PropertyGroup({"target", "entityReference"}, features = "reference") reference @PropertyGroup({"target", "entityReference"}) type @PropertyGroup("target", features = "reference") document <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< * PropertyGroup define the hierarchy (also proposed a String[] instead of String based value to show all possible ways to pass the hierarchy value) * PropertyFeature (name is negotiable :)) or PropertyGroup "features" field associate the group with a set of unique "features". This is the same logic than for extensions where several groups with with a shared feature are in conflict We could also decide to support only one feature per group right now since we don't yet have the need for several but it felt more natural like this. On Thu, Nov 15, 2018 at 8:04 AM Vincent Massol <vincent@massol.net> wrote:

...

On 15 Nov 2018, at 08:02, Vincent Massol <vincent@massol.net> wrote:

...

On 15 Nov 2018, at 06:29, Marius Dumitru Florea <mariusdumitru.florea@xwiki.com> wrote:

On Wed, Nov 14, 2018 at 5:12 PM Vincent Massol <vincent@massol.net> wrote:

...

I thought about something like this but I discarded it as I find this complicated for something that should be relatively simple.

I don't think it's that complicated because:

* Conflicting parameters should be an exception, not the rule. What other macros, besides include / display, need this? * If you just want to group macro parameters for display then you only need to use the @Group annotation. You don't need to implement a ParameterGroup. The ParameterGroup is needed only for conflicting parameters (ATM).

Sure but it’s still 10x more complicated than just having everything in one place in the parameters class with annotations as was suggested initially.

And requires unnecessary component instances that will stay in the EM for no need. The way to describe the descriptor is transient and only serves to generate the macro descriptors. In the end what’s important is the descriptor format.

Thanks -Vincent

...

Thanks -Vincent

...

Thanks, Marius

...

I’d prefer to have some simple annotations if possible. In other words, if feels a bit of over-engineering for the need. Now I have to admit that I stopped following this thread after the original proposal so maybe I’m just completely off :)

Thanks -Vincent

...

On 14 Nov 2018, at 15:51, Marius Dumitru Florea < mariusdumitru.florea@xwiki.com> wrote:

WDYT about:

-----8<----- IncludeMacroParameters ---------- @Group("target") page

@Group("target/entityReference") reference

@Group("target/entityReference") type

@Group("target") document

section

context ----->8---------------

That is: specify *only* the group hierarchy in the macro parameter descriptor. This would produce the following hierarchy:

* <target> ** page ** <entityReference> *** reference *** type ** document * section * context

Next, for the cases where we want to customize the behavior of a group, we introduce a component role ParameterGroup. For instance, for the "target" parameter group of the Include Macro we would create

@Named("include/target") public class TargetParameterGroup implements ParameterGroup {}

To specify that the members of a parameter group are exclusive we can either use a method in the ParameterGroup interface (e.g. isExclusive()) or use an annotation on the implementation TargetParameterGroup.

Thanks, Marius

On Tue, Nov 13, 2018 at 12:03 PM Adel Atallah <adel.atallah@xwiki.com> wrote:

...

Hello,

I'd like to briefly summarize the situation so that we can make some progress.

What we have: * We define "parameters" in a macro by creating a Java Bean, which provides all the getters and setters of the parameters we want. * We can use annotations on these getters/setters to define some behavior or metadata for these parameters (description, mandatory, deprecated...)

What we want: * Being able to handle conflicting parameters. For instance when we deprecate a parameter and add a new one to replace it, we should be able to either use the deprecated parameter or the new one but not both. * We also want to group parameters that are related to each other.

What we proposed: * Use annotations on the parameters to express the conflict. * Marius proposed to see the problem as a boolean expression such as: (page XOR (reference AND type) XOR document) OR section OR context. This would translate as: the user can use the 'section' and/or 'context' parameters (if they want), can use only one of these parameters: 'page', ('reference' and 'type') or 'document', where 'reference' and 'type' depend on each other and you can't use one without the other. * You can see on previous e-mails the kind of annotations we proposed to solve the issue.

Thanks, Adel

-- Thomas Mortagne

On Thu, Nov 15, 2018 at 10:23 AM Marius Dumitru Florea <mariusdumitru.florea@xwiki.com> wrote:

On Thu, Nov 15, 2018 at 10:51 AM Thomas Mortagne <thomas.mortagne@xwiki.com> wrote:

...

I'm also really not a fan of having to implement a component just to indicate that two groups of properties are conflicting.

+1 for making @Group support a hierarchy, that's indeed nice.

For for conflicting we need a dedicated annotation IMO.

So starting from your previous example I would expect something like:

...

...

...

...

>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>

@PropertyGroup("target") @PropertyFeature("reference") page

@PropertyGroup({"target", "entityReference"}) @PropertyFeature("reference") reference

@PropertyGroup({"target", "entityReference"}) type

@PropertyGroup("target") @PropertyFeature("reference") document

<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<

I don't think this is complete. The following doesn't make sense:

Actually you missed one point: the features are associated to the group. But I wrote the example too quickly, here is a fixed one: @PropertyGroup({"target", "page"}) @PropertyFeature("reference") page @PropertyGroup({"target", "entityReference"}) @PropertyFeature("reference") reference @PropertyGroup({"target", "entityReference"}) type @PropertyGroup("target", "reference") @PropertyFeature("reference") document

{{include page="..." type="..."/}}

No because page conflict with the whole target/entityReference group.

and neither this:

{{include document="..." type="..." /}}

No because document conflict with the whole target/entityReference group.

So it's not the reference parameter alone that provides the "reference" feature. The pair / group of parameters (reference and type) are providing the "reference" feature. This is why I think there is the need to specify the "feature" on the sub group "entityReference" not on the parameter. And to do this we need another class..

...

or

...

...

...

...

>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>

@PropertyGroup("target", features = "reference") page

@PropertyGroup({"target", "entityReference"}, features = "reference") reference

@PropertyGroup({"target", "entityReference"}) type

@PropertyGroup("target", features = "reference") document

<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<

...

* PropertyGroup define the hierarchy (also proposed a String[] instead of String based value to show all possible ways to pass the hierarchy value)

+1 for this

...

* PropertyFeature (name is negotiable :)) or PropertyGroup "features" field associate the group with a set of unique "features". This is the same logic than for extensions where several groups with with a shared feature are in conflict

You're not associating the feature to the group. That is the problem IMO. You are associating the feature to the parameter. For instance:

@PropertyGroup("foo", features = "input") one

@PropertyGroup("foo", features = "output") two

Is the "input" and "output" feature associate to the "foo" group or to the parameters one and two respectively?

Thanks, Marius

...

We could also decide to support only one feature per group right now since we don't yet have the need for several but it felt more natural like this.

On Thu, Nov 15, 2018 at 8:04 AM Vincent Massol <vincent@massol.net> wrote:

...

...

On 15 Nov 2018, at 08:02, Vincent Massol <vincent@massol.net> wrote:

...

On 15 Nov 2018, at 06:29, Marius Dumitru Florea <

mariusdumitru.florea@xwiki.com> wrote:

...

...

...

On Wed, Nov 14, 2018 at 5:12 PM Vincent Massol <vincent@massol.net>

wrote:

...

...

I thought about something like this but I discarded it as I find this complicated for something that should be relatively simple.

I don't think it's that complicated because:

* Conflicting parameters should be an exception, not the rule. What

other

...

macros, besides include / display, need this? * If you just want to group macro parameters for display then you only need to use the @Group annotation. You don't need to implement a ParameterGroup. The ParameterGroup is needed only for conflicting parameters (ATM).

Sure but it’s still 10x more complicated than just having everything in one place in the parameters class with annotations as was suggested initially.

And requires unnecessary component instances that will stay in the EM for no need. The way to describe the descriptor is transient and only serves to generate the macro descriptors. In the end what’s important is the descriptor format.

Thanks -Vincent

...

Thanks -Vincent

...

Thanks, Marius

...

I’d prefer to have some simple annotations if possible. In other

words, if

...

...

...

feels a bit of over-engineering for the need. Now I have to admit that I stopped following this thread after the original proposal so maybe I’m just completely off :)

Thanks -Vincent

> On 14 Nov 2018, at 15:51, Marius Dumitru Florea < mariusdumitru.florea@xwiki.com> wrote: > > WDYT about: > > -----8<----- IncludeMacroParameters ---------- > @Group("target") > page > > @Group("target/entityReference") > reference > > @Group("target/entityReference") > type > > @Group("target") > document > > section > > context > ----->8--------------- > > That is: specify *only* the group hierarchy in the macro parameter > descriptor. This would produce the following hierarchy: > > * <target> > ** page > ** <entityReference> > *** reference > *** type > ** document > * section > * context > > Next, for the cases where we want to customize the behavior of a group, we > introduce a component role ParameterGroup. For instance, for the "target" > parameter group of the Include Macro we would create > > @Named("include/target") > public class TargetParameterGroup implements ParameterGroup {} > > To specify that the members of a parameter group are exclusive we can > either use a method in the ParameterGroup interface (e.g. isExclusive()) or > use an annotation on the implementation TargetParameterGroup. > > Thanks, > Marius > > > On Tue, Nov 13, 2018 at 12:03 PM Adel Atallah < adel.atallah@xwiki.com> > wrote: > >> Hello, >> >> I'd like to briefly summarize the situation so that we can make some >> progress. >> >> What we have: >> * We define "parameters" in a macro by creating a Java Bean, which >> provides all the getters and setters of the parameters we want. >> * We can use annotations on these getters/setters to define some >> behavior or metadata for these parameters (description, mandatory, >> deprecated...) >> >> What we want: >> * Being able to handle conflicting parameters. For instance when we >> deprecate a parameter and add a new one to replace it, we should be >> able to either use the deprecated parameter or the new one but not >> both. >> * We also want to group parameters that are related to each other. >> >> What we proposed: >> * Use annotations on the parameters to express the conflict. >> * Marius proposed to see the problem as a boolean expression such as: >> (page XOR (reference AND type) XOR document) OR section OR context. >> This would translate as: the user can use the 'section' and/or >> 'context' parameters (if they want), can use only one of these >> parameters: 'page', ('reference' and 'type') or 'document', where >> 'reference' and 'type' depend on each other and you can't use one >> without the other. >> * You can see on previous e-mails the kind of annotations we proposed >> to solve the issue. >> >> Thanks, >> Adel

-- Thomas Mortagne

-- Thomas Mortagne

Ok so it seems like we are getting back to the proposition we made with Vincent. We need one annotation to enforce the dependence between parameters (reference and type in our example) and another one that can be used to *deduce* conflicting parameters. I don't understand how a hierarchy of groups can help us specify a dependence between parameters. A parameter is either in the same group as another one or it is not. The hierarchy seems to focus on problems that we are not trying to solve here. The original proposal was similar to what Thomas proposed, but without hierarchy: @Alternative("reference") @Group("entityReference") reference @Alternative("reference") @Group("entityReference") type @Alternative("reference") page @Alternative("reference") document where "Alternative" is the same as "Feature". Now Marius didn't agree with that because the "Alternative" annotation should not be bind to "reference" and "type" parameters but to the group "entityReference", which is not possible to do without creating other classes. I don't think this is an issue to put the "Alternative" annotation on "reference" and "type" because we should have all the necessary information to *deduce* the conflicting parameters. It's true that removing the "Alternative" annotation of one of "reference" or "type" should produce the same result though, which could be confusing. On Thu, Nov 15, 2018 at 10:23 AM Marius Dumitru Florea <mariusdumitru.florea@xwiki.com> wrote:

On Thu, Nov 15, 2018 at 10:51 AM Thomas Mortagne <thomas.mortagne@xwiki.com> wrote:

...

I'm also really not a fan of having to implement a component just to indicate that two groups of properties are conflicting.

+1 for making @Group support a hierarchy, that's indeed nice.

For for conflicting we need a dedicated annotation IMO.

So starting from your previous example I would expect something like:

...

...

...

...

>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>

@PropertyGroup("target") @PropertyFeature("reference") page

@PropertyGroup({"target", "entityReference"}) @PropertyFeature("reference") reference

@PropertyGroup({"target", "entityReference"}) type

@PropertyGroup("target") @PropertyFeature("reference") document

<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<

I don't think this is complete. The following doesn't make sense:

{{include page="..." type="..."/}}

and neither this:

{{include document="..." type="..." /}}

So it's not the reference parameter alone that provides the "reference" feature. The pair / group of parameters (reference and type) are providing the "reference" feature. This is why I think there is the need to specify the "feature" on the sub group "entityReference" not on the parameter. And to do this we need another class..

...

or

...

...

...

...

>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>

@PropertyGroup("target", features = "reference") page

@PropertyGroup({"target", "entityReference"}, features = "reference") reference

@PropertyGroup({"target", "entityReference"}) type

@PropertyGroup("target", features = "reference") document

<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<

...

* PropertyGroup define the hierarchy (also proposed a String[] instead of String based value to show all possible ways to pass the hierarchy value)

+1 for this

...

* PropertyFeature (name is negotiable :)) or PropertyGroup "features" field associate the group with a set of unique "features". This is the same logic than for extensions where several groups with with a shared feature are in conflict

You're not associating the feature to the group. That is the problem IMO. You are associating the feature to the parameter. For instance:

@PropertyGroup("foo", features = "input") one

@PropertyGroup("foo", features = "output") two

Is the "input" and "output" feature associate to the "foo" group or to the parameters one and two respectively?

Thanks, Marius

...

We could also decide to support only one feature per group right now since we don't yet have the need for several but it felt more natural like this.

On Thu, Nov 15, 2018 at 8:04 AM Vincent Massol <vincent@massol.net> wrote:

...

...

On 15 Nov 2018, at 08:02, Vincent Massol <vincent@massol.net> wrote:

...

On 15 Nov 2018, at 06:29, Marius Dumitru Florea <

mariusdumitru.florea@xwiki.com> wrote:

...

...

...

On Wed, Nov 14, 2018 at 5:12 PM Vincent Massol <vincent@massol.net>

wrote:

...

...

I thought about something like this but I discarded it as I find this complicated for something that should be relatively simple.

I don't think it's that complicated because:

* Conflicting parameters should be an exception, not the rule. What

other

...

macros, besides include / display, need this? * If you just want to group macro parameters for display then you only need to use the @Group annotation. You don't need to implement a ParameterGroup. The ParameterGroup is needed only for conflicting parameters (ATM).

Sure but it’s still 10x more complicated than just having everything in one place in the parameters class with annotations as was suggested initially.

And requires unnecessary component instances that will stay in the EM for no need. The way to describe the descriptor is transient and only serves to generate the macro descriptors. In the end what’s important is the descriptor format.

Thanks -Vincent

...

Thanks -Vincent

...

Thanks, Marius

...

I’d prefer to have some simple annotations if possible. In other

words, if

...

...

...

feels a bit of over-engineering for the need. Now I have to admit that I stopped following this thread after the original proposal so maybe I’m just completely off :)

Thanks -Vincent

> On 14 Nov 2018, at 15:51, Marius Dumitru Florea < mariusdumitru.florea@xwiki.com> wrote: > > WDYT about: > > -----8<----- IncludeMacroParameters ---------- > @Group("target") > page > > @Group("target/entityReference") > reference > > @Group("target/entityReference") > type > > @Group("target") > document > > section > > context > ----->8--------------- > > That is: specify *only* the group hierarchy in the macro parameter > descriptor. This would produce the following hierarchy: > > * <target> > ** page > ** <entityReference> > *** reference > *** type > ** document > * section > * context > > Next, for the cases where we want to customize the behavior of a group, we > introduce a component role ParameterGroup. For instance, for the "target" > parameter group of the Include Macro we would create > > @Named("include/target") > public class TargetParameterGroup implements ParameterGroup {} > > To specify that the members of a parameter group are exclusive we can > either use a method in the ParameterGroup interface (e.g. isExclusive()) or > use an annotation on the implementation TargetParameterGroup. > > Thanks, > Marius > > > On Tue, Nov 13, 2018 at 12:03 PM Adel Atallah < adel.atallah@xwiki.com> > wrote: > >> Hello, >> >> I'd like to briefly summarize the situation so that we can make some >> progress. >> >> What we have: >> * We define "parameters" in a macro by creating a Java Bean, which >> provides all the getters and setters of the parameters we want. >> * We can use annotations on these getters/setters to define some >> behavior or metadata for these parameters (description, mandatory, >> deprecated...) >> >> What we want: >> * Being able to handle conflicting parameters. For instance when we >> deprecate a parameter and add a new one to replace it, we should be >> able to either use the deprecated parameter or the new one but not >> both. >> * We also want to group parameters that are related to each other. >> >> What we proposed: >> * Use annotations on the parameters to express the conflict. >> * Marius proposed to see the problem as a boolean expression such as: >> (page XOR (reference AND type) XOR document) OR section OR context. >> This would translate as: the user can use the 'section' and/or >> 'context' parameters (if they want), can use only one of these >> parameters: 'page', ('reference' and 'type') or 'document', where >> 'reference' and 'type' depend on each other and you can't use one >> without the other. >> * You can see on previous e-mails the kind of annotations we proposed >> to solve the issue. >> >> Thanks, >> Adel

-- Thomas Mortagne

On Thu, Nov 15, 2018 at 11:13 AM Thomas Mortagne <thomas.mortagne@xwiki.com> wrote:

On Thu, Nov 15, 2018 at 11:06 AM Adel Atallah <adel.atallah@xwiki.com> wrote:

...

Ok so it seems like we are getting back to the proposition we made with Vincent. We need one annotation to enforce the dependence between parameters (reference and type in our example) and another one that can be used to *deduce* conflicting parameters.

...

I don't understand how a hierarchy of groups can help us specify a dependence between parameters.

I don't think it does, it's just that since we are defining groups having subgroups would be useful visually.

...

A parameter is either in the same group as another one or it is not. The hierarchy seems to focus on problems that we are not trying to solve here. The original proposal was similar to what Thomas proposed, but without hierarchy:

@Alternative("reference") @Group("entityReference") reference

@Alternative("reference") @Group("entityReference") type

@Alternative("reference") page

@Alternative("reference") document

where "Alternative" is the same as "Feature". Now Marius didn't agree with that because the "Alternative" annotation should not be bind to "reference" and "type" parameters but to the group "entityReference"

And as I said in my proposal the features are associated to the group, not the properties. I agree that associating it to the property (and ending up with half of a group conflicting with half of another) does really make sense.

This is not enforced by the code. You know that features are associated to groups *because* they are bound to the same property. Anyway I'm +1 to do it this way.

...

, which is not possible to do without creating other classes. I don't think this is an issue to put the "Alternative" annotation on "reference" and "type" because we should have all the necessary information to *deduce* the conflicting parameters. It's true that removing the "Alternative" annotation of one of "reference" or "type" should produce the same result though, which could be confusing. On Thu, Nov 15, 2018 at 10:23 AM Marius Dumitru Florea <mariusdumitru.florea@xwiki.com> wrote:

...

On Thu, Nov 15, 2018 at 10:51 AM Thomas Mortagne <thomas.mortagne@xwiki.com> wrote:

...

I'm also really not a fan of having to implement a component just to indicate that two groups of properties are conflicting.

+1 for making @Group support a hierarchy, that's indeed nice.

For for conflicting we need a dedicated annotation IMO.

So starting from your previous example I would expect something like:

...

...

>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>

@PropertyGroup("target") @PropertyFeature("reference") page

@PropertyGroup({"target", "entityReference"}) @PropertyFeature("reference") reference

@PropertyGroup({"target", "entityReference"}) type

@PropertyGroup("target") @PropertyFeature("reference") document

<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<

I don't think this is complete. The following doesn't make sense:

{{include page="..." type="..."/}}

and neither this:

{{include document="..." type="..." /}}

So it's not the reference parameter alone that provides the "reference" feature. The pair / group of parameters (reference and type) are providing the "reference" feature. This is why I think there is the need to specify the "feature" on the sub group "entityReference" not on the parameter. And to do this we need another class..

...

or

...

...

>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>

@PropertyGroup("target", features = "reference") page

@PropertyGroup({"target", "entityReference"}, features = "reference") reference

@PropertyGroup({"target", "entityReference"}) type

@PropertyGroup("target", features = "reference") document

<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<

...

* PropertyGroup define the hierarchy (also proposed a String[] instead of String based value to show all possible ways to pass the hierarchy value)

+1 for this

...

* PropertyFeature (name is negotiable :)) or PropertyGroup "features" field associate the group with a set of unique "features". This is the same logic than for extensions where several groups with with a shared feature are in conflict

You're not associating the feature to the group. That is the problem IMO. You are associating the feature to the parameter. For instance:

@PropertyGroup("foo", features = "input") one

@PropertyGroup("foo", features = "output") two

Is the "input" and "output" feature associate to the "foo" group or to the parameters one and two respectively?

Thanks, Marius

...

We could also decide to support only one feature per group right now since we don't yet have the need for several but it felt more natural like this.

On Thu, Nov 15, 2018 at 8:04 AM Vincent Massol <vincent@massol.net> wrote:

...

...

On 15 Nov 2018, at 08:02, Vincent Massol <vincent@massol.net> wrote:

> On 15 Nov 2018, at 06:29, Marius Dumitru Florea <

mariusdumitru.florea@xwiki.com> wrote:

...

...

> > On Wed, Nov 14, 2018 at 5:12 PM Vincent Massol <vincent@massol.net> wrote: > >> I thought about something like this but I discarded it as I find this >> complicated for something that should be relatively simple. > > > I don't think it's that complicated because: > > * Conflicting parameters should be an exception, not the rule. What other > macros, besides include / display, need this? > * If you just want to group macro parameters for display then you only need > to use the @Group annotation. You don't need to implement a ParameterGroup. > The ParameterGroup is needed only for conflicting parameters (ATM).

Sure but it’s still 10x more complicated than just having everything in one place in the parameters class with annotations as was suggested initially.

And requires unnecessary component instances that will stay in the EM for no need. The way to describe the descriptor is transient and only serves to generate the macro descriptors. In the end what’s important is the descriptor format.

Thanks -Vincent

...

Thanks -Vincent

> > Thanks, > Marius > > >> I’d prefer to have some simple annotations if possible. In other

words, if

...

>> feels a bit of over-engineering for the need. Now I have to admit that I >> stopped following this thread after the original proposal so maybe I’m just >> completely off :) >> >> Thanks >> -Vincent >> >>> On 14 Nov 2018, at 15:51, Marius Dumitru Florea < >> mariusdumitru.florea@xwiki.com> wrote: >>> >>> WDYT about: >>> >>> -----8<----- IncludeMacroParameters ---------- >>> @Group("target") >>> page >>> >>> @Group("target/entityReference") >>> reference >>> >>> @Group("target/entityReference") >>> type >>> >>> @Group("target") >>> document >>> >>> section >>> >>> context >>> ----->8--------------- >>> >>> That is: specify *only* the group hierarchy in the macro parameter >>> descriptor. This would produce the following hierarchy: >>> >>> * <target> >>> ** page >>> ** <entityReference> >>> *** reference >>> *** type >>> ** document >>> * section >>> * context >>> >>> Next, for the cases where we want to customize the behavior of a group, >> we >>> introduce a component role ParameterGroup. For instance, for the "target" >>> parameter group of the Include Macro we would create >>> >>> @Named("include/target") >>> public class TargetParameterGroup implements ParameterGroup {} >>> >>> To specify that the members of a parameter group are exclusive we can >>> either use a method in the ParameterGroup interface (e.g. isExclusive()) >> or >>> use an annotation on the implementation TargetParameterGroup. >>> >>> Thanks, >>> Marius >>> >>> >>> On Tue, Nov 13, 2018 at 12:03 PM Adel Atallah < adel.atallah@xwiki.com> >>> wrote: >>> >>>> Hello, >>>> >>>> I'd like to briefly summarize the situation so that we can make some >>>> progress. >>>> >>>> What we have: >>>> * We define "parameters" in a macro by creating a Java Bean, which >>>> provides all the getters and setters of the parameters we want. >>>> * We can use annotations on these getters/setters to define some >>>> behavior or metadata for these parameters (description, mandatory, >>>> deprecated...) >>>> >>>> What we want: >>>> * Being able to handle conflicting parameters. For instance when we >>>> deprecate a parameter and add a new one to replace it, we should be >>>> able to either use the deprecated parameter or the new one but not >>>> both. >>>> * We also want to group parameters that are related to each other. >>>> >>>> What we proposed: >>>> * Use annotations on the parameters to express the conflict. >>>> * Marius proposed to see the problem as a boolean expression such as: >>>> (page XOR (reference AND type) XOR document) OR section OR context. >>>> This would translate as: the user can use the 'section' and/or >>>> 'context' parameters (if they want), can use only one of these >>>> parameters: 'page', ('reference' and 'type') or 'document', where >>>> 'reference' and 'type' depend on each other and you can't use one >>>> without the other. >>>> * You can see on previous e-mails the kind of annotations we proposed >>>> to solve the issue. >>>> >>>> Thanks, >>>> Adel

-- Thomas Mortagne

-- Thomas Mortagne

On Thu, Nov 15, 2018 at 1:56 PM Thomas Mortagne <thomas.mortagne@xwiki.com> wrote:

On Thu, Nov 15, 2018 at 1:24 PM Adel Atallah <adel.atallah@xwiki.com> wrote:

...

On Thu, Nov 15, 2018 at 11:13 AM Thomas Mortagne <thomas.mortagne@xwiki.com> wrote:

...

On Thu, Nov 15, 2018 at 11:06 AM Adel Atallah <adel.atallah@xwiki.com> wrote:

...

Ok so it seems like we are getting back to the proposition we made with Vincent. We need one annotation to enforce the dependence between parameters (reference and type in our example) and another one that can be used to *deduce* conflicting parameters.

...

I don't understand how a hierarchy of groups can help us specify a dependence between parameters.

I don't think it does, it's just that since we are defining groups having subgroups would be useful visually.

...

A parameter is either in the same group as another one or it is not. The hierarchy seems to focus on problems that we are not trying to solve here. The original proposal was similar to what Thomas proposed, but without hierarchy:

@Alternative("reference") @Group("entityReference") reference

@Alternative("reference") @Group("entityReference") type

@Alternative("reference") page

@Alternative("reference") document

where "Alternative" is the same as "Feature". Now Marius didn't agree with that because the "Alternative" annotation should not be bind to "reference" and "type" parameters but to the group "entityReference"

And as I said in my proposal the features are associated to the group, not the properties. I agree that associating it to the property (and ending up with half of a group conflicting with half of another) does really make sense.

This is not enforced by the code.

I don't understand what you mean, there is no code yet.

By code I meant the annotations in the code.

For me the code which is going to parse this Java bean will of course make sure the features are associated to the group of the property.

I agree with that.

...

You know that features are associated to groups *because* they are bound to the same property. Anyway I'm +1 to do it this way.

...

...

, which is not possible to do without creating other classes. I don't think this is an issue to put the "Alternative" annotation on "reference" and "type" because we should have all the necessary information to *deduce* the conflicting parameters. It's true that removing the "Alternative" annotation of one of "reference" or "type" should produce the same result though, which could be confusing. On Thu, Nov 15, 2018 at 10:23 AM Marius Dumitru Florea <mariusdumitru.florea@xwiki.com> wrote:

...

On Thu, Nov 15, 2018 at 10:51 AM Thomas Mortagne <thomas.mortagne@xwiki.com> wrote:

...

I'm also really not a fan of having to implement a component just to indicate that two groups of properties are conflicting.

+1 for making @Group support a hierarchy, that's indeed nice.

For for conflicting we need a dedicated annotation IMO.

So starting from your previous example I would expect something like:

>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>

@PropertyGroup("target") @PropertyFeature("reference") page

@PropertyGroup({"target", "entityReference"}) @PropertyFeature("reference") reference

@PropertyGroup({"target", "entityReference"}) type

@PropertyGroup("target") @PropertyFeature("reference") document

<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<

I don't think this is complete. The following doesn't make sense:

{{include page="..." type="..."/}}

and neither this:

{{include document="..." type="..." /}}

So it's not the reference parameter alone that provides the "reference" feature. The pair / group of parameters (reference and type) are providing the "reference" feature. This is why I think there is the need to specify the "feature" on the sub group "entityReference" not on the parameter. And to do this we need another class..

...

or

>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>

@PropertyGroup("target", features = "reference") page

@PropertyGroup({"target", "entityReference"}, features = "reference") reference

@PropertyGroup({"target", "entityReference"}) type

@PropertyGroup("target", features = "reference") document

<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<

...

* PropertyGroup define the hierarchy (also proposed a String[] instead of String based value to show all possible ways to pass the hierarchy value)

+1 for this

...

* PropertyFeature (name is negotiable :)) or PropertyGroup "features" field associate the group with a set of unique "features". This is the same logic than for extensions where several groups with with a shared feature are in conflict

You're not associating the feature to the group. That is the problem IMO. You are associating the feature to the parameter. For instance:

@PropertyGroup("foo", features = "input") one

@PropertyGroup("foo", features = "output") two

Is the "input" and "output" feature associate to the "foo" group or to the parameters one and two respectively?

Thanks, Marius

...

We could also decide to support only one feature per group right now since we don't yet have the need for several but it felt more natural like this.

On Thu, Nov 15, 2018 at 8:04 AM Vincent Massol <vincent@massol.net> wrote: > > > > > On 15 Nov 2018, at 08:02, Vincent Massol <vincent@massol.net> wrote: > > > > > > > >> On 15 Nov 2018, at 06:29, Marius Dumitru Florea < mariusdumitru.florea@xwiki.com> wrote: > >> > >> On Wed, Nov 14, 2018 at 5:12 PM Vincent Massol <vincent@massol.net> wrote: > >> > >>> I thought about something like this but I discarded it as I find this > >>> complicated for something that should be relatively simple. > >> > >> > >> I don't think it's that complicated because: > >> > >> * Conflicting parameters should be an exception, not the rule. What other > >> macros, besides include / display, need this? > >> * If you just want to group macro parameters for display then you only need > >> to use the @Group annotation. You don't need to implement a ParameterGroup. > >> The ParameterGroup is needed only for conflicting parameters (ATM). > > > > Sure but it’s still 10x more complicated than just having everything in one place in the parameters class with annotations as was suggested initially. > > And requires unnecessary component instances that will stay in the EM for no need. The way to describe the descriptor is transient and only serves to generate the macro descriptors. In the end what’s important is the descriptor format. > > Thanks > -Vincent > > > > > Thanks > > -Vincent > > > >> > >> Thanks, > >> Marius > >> > >> > >>> I’d prefer to have some simple annotations if possible. In other words, if > >>> feels a bit of over-engineering for the need. Now I have to admit that I > >>> stopped following this thread after the original proposal so maybe I’m just > >>> completely off :) > >>> > >>> Thanks > >>> -Vincent > >>> > >>>> On 14 Nov 2018, at 15:51, Marius Dumitru Florea < > >>> mariusdumitru.florea@xwiki.com> wrote: > >>>> > >>>> WDYT about: > >>>> > >>>> -----8<----- IncludeMacroParameters ---------- > >>>> @Group("target") > >>>> page > >>>> > >>>> @Group("target/entityReference") > >>>> reference > >>>> > >>>> @Group("target/entityReference") > >>>> type > >>>> > >>>> @Group("target") > >>>> document > >>>> > >>>> section > >>>> > >>>> context > >>>> ----->8--------------- > >>>> > >>>> That is: specify *only* the group hierarchy in the macro parameter > >>>> descriptor. This would produce the following hierarchy: > >>>> > >>>> * <target> > >>>> ** page > >>>> ** <entityReference> > >>>> *** reference > >>>> *** type > >>>> ** document > >>>> * section > >>>> * context > >>>> > >>>> Next, for the cases where we want to customize the behavior of a group, > >>> we > >>>> introduce a component role ParameterGroup. For instance, for the "target" > >>>> parameter group of the Include Macro we would create > >>>> > >>>> @Named("include/target") > >>>> public class TargetParameterGroup implements ParameterGroup {} > >>>> > >>>> To specify that the members of a parameter group are exclusive we can > >>>> either use a method in the ParameterGroup interface (e.g. isExclusive()) > >>> or > >>>> use an annotation on the implementation TargetParameterGroup. > >>>> > >>>> Thanks, > >>>> Marius > >>>> > >>>> > >>>> On Tue, Nov 13, 2018 at 12:03 PM Adel Atallah < adel.atallah@xwiki.com> > >>>> wrote: > >>>> > >>>>> Hello, > >>>>> > >>>>> I'd like to briefly summarize the situation so that we can make some > >>>>> progress. > >>>>> > >>>>> What we have: > >>>>> * We define "parameters" in a macro by creating a Java Bean, which > >>>>> provides all the getters and setters of the parameters we want. > >>>>> * We can use annotations on these getters/setters to define some > >>>>> behavior or metadata for these parameters (description, mandatory, > >>>>> deprecated...) > >>>>> > >>>>> What we want: > >>>>> * Being able to handle conflicting parameters. For instance when we > >>>>> deprecate a parameter and add a new one to replace it, we should be > >>>>> able to either use the deprecated parameter or the new one but not > >>>>> both. > >>>>> * We also want to group parameters that are related to each other. > >>>>> > >>>>> What we proposed: > >>>>> * Use annotations on the parameters to express the conflict. > >>>>> * Marius proposed to see the problem as a boolean expression such as: > >>>>> (page XOR (reference AND type) XOR document) OR section OR context. > >>>>> This would translate as: the user can use the 'section' and/or > >>>>> 'context' parameters (if they want), can use only one of these > >>>>> parameters: 'page', ('reference' and 'type') or 'document', where > >>>>> 'reference' and 'type' depend on each other and you can't use one > >>>>> without the other. > >>>>> * You can see on previous e-mails the kind of annotations we proposed > >>>>> to solve the issue. > >>>>> > >>>>> Thanks, > >>>>> Adel >

-- Thomas Mortagne

-- Thomas Mortagne

-- Thomas Mortagne

So do we agree on trying the solution given by Thomas? i.e. creating two annotations: 1. PropertyGroup to specify a hierarchy of groups to a parameter 2. PropertyFeature to indicate that some parameters/groups represents the same feature (which can lead to conflicts). Here was the given example:

...

...

...

...

...

>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>

@PropertyGroup("target") @PropertyFeature("reference") page @PropertyGroup({"target", "entityReference"}) @PropertyFeature("reference") reference @PropertyGroup({"target", "entityReference"}) type @PropertyGroup("target") @PropertyFeature("reference") document <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< I've already started implementing the PropertyGroup annotation. On Thu, Nov 15, 2018 at 3:13 PM Thomas Mortagne <thomas.mortagne@xwiki.com> wrote:

On Thu, Nov 15, 2018 at 2:06 PM Adel Atallah <adel.atallah@xwiki.com> wrote:

...

On Thu, Nov 15, 2018 at 1:56 PM Thomas Mortagne <thomas.mortagne@xwiki.com> wrote:

...

On Thu, Nov 15, 2018 at 1:24 PM Adel Atallah <adel.atallah@xwiki.com> wrote:

...

On Thu, Nov 15, 2018 at 11:13 AM Thomas Mortagne <thomas.mortagne@xwiki.com> wrote:

...

On Thu, Nov 15, 2018 at 11:06 AM Adel Atallah <adel.atallah@xwiki.com> wrote:

...

Ok so it seems like we are getting back to the proposition we made with Vincent. We need one annotation to enforce the dependence between parameters (reference and type in our example) and another one that can be used to *deduce* conflicting parameters.

...

I don't understand how a hierarchy of groups can help us specify a dependence between parameters.

I don't think it does, it's just that since we are defining groups having subgroups would be useful visually.

...

A parameter is either in the same group as another one or it is not. The hierarchy seems to focus on problems that we are not trying to solve here. The original proposal was similar to what Thomas proposed, but without hierarchy:

@Alternative("reference") @Group("entityReference") reference

@Alternative("reference") @Group("entityReference") type

@Alternative("reference") page

@Alternative("reference") document

where "Alternative" is the same as "Feature". Now Marius didn't agree with that because the "Alternative" annotation should not be bind to "reference" and "type" parameters but to the group "entityReference"

And as I said in my proposal the features are associated to the group, not the properties. I agree that associating it to the property (and ending up with half of a group conflicting with half of another) does really make sense.

This is not enforced by the code.

I don't understand what you mean, there is no code yet.

By code I meant the annotations in the code.

Yes but hard to do much better I think without breaking anything now that we have two parameters for a single information, we need to maintain them.

...

...

For me the code which is going to parse this Java bean will of course make sure the features are associated to the group of the property.

I agree with that.

...

...

You know that features are associated to groups *because* they are bound to the same property. Anyway I'm +1 to do it this way.

...

...

, which is not possible to do without creating other classes. I don't think this is an issue to put the "Alternative" annotation on "reference" and "type" because we should have all the necessary information to *deduce* the conflicting parameters. It's true that removing the "Alternative" annotation of one of "reference" or "type" should produce the same result though, which could be confusing. On Thu, Nov 15, 2018 at 10:23 AM Marius Dumitru Florea <mariusdumitru.florea@xwiki.com> wrote: > > On Thu, Nov 15, 2018 at 10:51 AM Thomas Mortagne <thomas.mortagne@xwiki.com> > wrote: > > > I'm also really not a fan of having to implement a component just to > > indicate that two groups of properties are conflicting. > > > > +1 for making @Group support a hierarchy, that's indeed nice. > > > > For for conflicting we need a dedicated annotation IMO. > > > > So starting from your previous example I would expect something like: > > > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > > > > @PropertyGroup("target") > > @PropertyFeature("reference") > > page > > > > @PropertyGroup({"target", "entityReference"}) > > @PropertyFeature("reference") > > reference > > > > @PropertyGroup({"target", "entityReference"}) > > type > > > > @PropertyGroup("target") > > @PropertyFeature("reference") > > document > > > > <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< > > > > I don't think this is complete. The following doesn't make sense: > > {{include page="..." type="..."/}} > > and neither this: > > {{include document="..." type="..." /}} > > So it's not the reference parameter alone that provides the "reference" > feature. The pair / group of parameters (reference and type) are providing > the "reference" feature. This is why I think there is the need to specify > the "feature" on the sub group "entityReference" not on the parameter. And > to do this we need another class.. > > > > > > > or > > > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > > > > @PropertyGroup("target", features = "reference") > > page > > > > @PropertyGroup({"target", "entityReference"}, features = "reference") > > reference > > > > @PropertyGroup({"target", "entityReference"}) > > type > > > > @PropertyGroup("target", features = "reference") > > document > > > > <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< > > > > > > > * PropertyGroup define the hierarchy (also proposed a String[] instead > > of String based value to show all possible ways to pass the hierarchy > > value) > > > > +1 for this > > > > * PropertyFeature (name is negotiable :)) or PropertyGroup "features" > > field associate the group with a set of unique "features". This is the > > same logic than for extensions where several groups with with a shared > > feature are in conflict > > > > You're not associating the feature to the group. That is the problem IMO. > You are associating the feature to the parameter. For instance: > > @PropertyGroup("foo", features = "input") > one > > @PropertyGroup("foo", features = "output") > two > > Is the "input" and "output" feature associate to the "foo" group or to the > parameters one and two respectively? > > Thanks, > Marius > > > > > > We could also decide to support only one feature per group right now > > since we don't yet have the need for several but it felt more natural > > like this. > > > > On Thu, Nov 15, 2018 at 8:04 AM Vincent Massol <vincent@massol.net> wrote: > > > > > > > > > > > > > On 15 Nov 2018, at 08:02, Vincent Massol <vincent@massol.net> wrote: > > > > > > > > > > > > > > > >> On 15 Nov 2018, at 06:29, Marius Dumitru Florea < > > mariusdumitru.florea@xwiki.com> wrote: > > > >> > > > >> On Wed, Nov 14, 2018 at 5:12 PM Vincent Massol <vincent@massol.net> > > wrote: > > > >> > > > >>> I thought about something like this but I discarded it as I find this > > > >>> complicated for something that should be relatively simple. > > > >> > > > >> > > > >> I don't think it's that complicated because: > > > >> > > > >> * Conflicting parameters should be an exception, not the rule. What > > other > > > >> macros, besides include / display, need this? > > > >> * If you just want to group macro parameters for display then you > > only need > > > >> to use the @Group annotation. You don't need to implement a > > ParameterGroup. > > > >> The ParameterGroup is needed only for conflicting parameters (ATM). > > > > > > > > Sure but it’s still 10x more complicated than just having everything > > in one place in the parameters class with annotations as was suggested > > initially. > > > > > > And requires unnecessary component instances that will stay in the EM > > for no need. The way to describe the descriptor is transient and only > > serves to generate the macro descriptors. In the end what’s important is > > the descriptor format. > > > > > > Thanks > > > -Vincent > > > > > > > > > > > Thanks > > > > -Vincent > > > > > > > >> > > > >> Thanks, > > > >> Marius > > > >> > > > >> > > > >>> I’d prefer to have some simple annotations if possible. In other > > words, if > > > >>> feels a bit of over-engineering for the need. Now I have to admit > > that I > > > >>> stopped following this thread after the original proposal so maybe > > I’m just > > > >>> completely off :) > > > >>> > > > >>> Thanks > > > >>> -Vincent > > > >>> > > > >>>> On 14 Nov 2018, at 15:51, Marius Dumitru Florea < > > > >>> mariusdumitru.florea@xwiki.com> wrote: > > > >>>> > > > >>>> WDYT about: > > > >>>> > > > >>>> -----8<----- IncludeMacroParameters ---------- > > > >>>> @Group("target") > > > >>>> page > > > >>>> > > > >>>> @Group("target/entityReference") > > > >>>> reference > > > >>>> > > > >>>> @Group("target/entityReference") > > > >>>> type > > > >>>> > > > >>>> @Group("target") > > > >>>> document > > > >>>> > > > >>>> section > > > >>>> > > > >>>> context > > > >>>> ----->8--------------- > > > >>>> > > > >>>> That is: specify *only* the group hierarchy in the macro parameter > > > >>>> descriptor. This would produce the following hierarchy: > > > >>>> > > > >>>> * <target> > > > >>>> ** page > > > >>>> ** <entityReference> > > > >>>> *** reference > > > >>>> *** type > > > >>>> ** document > > > >>>> * section > > > >>>> * context > > > >>>> > > > >>>> Next, for the cases where we want to customize the behavior of a > > group, > > > >>> we > > > >>>> introduce a component role ParameterGroup. For instance, for the > > "target" > > > >>>> parameter group of the Include Macro we would create > > > >>>> > > > >>>> @Named("include/target") > > > >>>> public class TargetParameterGroup implements ParameterGroup {} > > > >>>> > > > >>>> To specify that the members of a parameter group are exclusive we > > can > > > >>>> either use a method in the ParameterGroup interface (e.g. > > isExclusive()) > > > >>> or > > > >>>> use an annotation on the implementation TargetParameterGroup. > > > >>>> > > > >>>> Thanks, > > > >>>> Marius > > > >>>> > > > >>>> > > > >>>> On Tue, Nov 13, 2018 at 12:03 PM Adel Atallah < > > adel.atallah@xwiki.com> > > > >>>> wrote: > > > >>>> > > > >>>>> Hello, > > > >>>>> > > > >>>>> I'd like to briefly summarize the situation so that we can make > > some > > > >>>>> progress. > > > >>>>> > > > >>>>> What we have: > > > >>>>> * We define "parameters" in a macro by creating a Java Bean, which > > > >>>>> provides all the getters and setters of the parameters we want. > > > >>>>> * We can use annotations on these getters/setters to define some > > > >>>>> behavior or metadata for these parameters (description, mandatory, > > > >>>>> deprecated...) > > > >>>>> > > > >>>>> What we want: > > > >>>>> * Being able to handle conflicting parameters. For instance when we > > > >>>>> deprecate a parameter and add a new one to replace it, we should be > > > >>>>> able to either use the deprecated parameter or the new one but not > > > >>>>> both. > > > >>>>> * We also want to group parameters that are related to each other. > > > >>>>> > > > >>>>> What we proposed: > > > >>>>> * Use annotations on the parameters to express the conflict. > > > >>>>> * Marius proposed to see the problem as a boolean expression such > > as: > > > >>>>> (page XOR (reference AND type) XOR document) OR section OR context. > > > >>>>> This would translate as: the user can use the 'section' and/or > > > >>>>> 'context' parameters (if they want), can use only one of these > > > >>>>> parameters: 'page', ('reference' and 'type') or 'document', where > > > >>>>> 'reference' and 'type' depend on each other and you can't use one > > > >>>>> without the other. > > > >>>>> * You can see on previous e-mails the kind of annotations we > > proposed > > > >>>>> to solve the issue. > > > >>>>> > > > >>>>> Thanks, > > > >>>>> Adel > > > > > > > > > -- > > Thomas Mortagne > >

-- Thomas Mortagne

-- Thomas Mortagne

-- Thomas Mortagne

Actually that's the wrong example, with that one you end up with page and document in the same group so they can't conflict :) The fixed one was: <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< @PropertyGroup({"target", "page"}) @PropertyFeature("reference") page @PropertyGroup({"target", "entityReference"}) @PropertyFeature("reference") reference @PropertyGroup({"target", "entityReference"}) type @PropertyGroup("target", "reference") @PropertyFeature("reference") document

...

...

...

...

...

>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>><

and properties not associated to a group being automatically part of a group with a unique property. On Fri, Nov 16, 2018 at 10:42 AM Adel Atallah <adel.atallah@xwiki.com> wrote:

So do we agree on trying the solution given by Thomas? i.e. creating two annotations: 1. PropertyGroup to specify a hierarchy of groups to a parameter 2. PropertyFeature to indicate that some parameters/groups represents the same feature (which can lead to conflicts).

Here was the given example:

...

...

...

...

...

>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>

@PropertyGroup("target") @PropertyFeature("reference") page

@PropertyGroup({"target", "entityReference"}) @PropertyFeature("reference") reference

@PropertyGroup({"target", "entityReference"}) type

@PropertyGroup("target") @PropertyFeature("reference") document

<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<

I've already started implementing the PropertyGroup annotation.

On Thu, Nov 15, 2018 at 3:13 PM Thomas Mortagne <thomas.mortagne@xwiki.com> wrote:

...

On Thu, Nov 15, 2018 at 2:06 PM Adel Atallah <adel.atallah@xwiki.com> wrote:

...

On Thu, Nov 15, 2018 at 1:56 PM Thomas Mortagne <thomas.mortagne@xwiki.com> wrote:

...

On Thu, Nov 15, 2018 at 1:24 PM Adel Atallah <adel.atallah@xwiki.com> wrote:

...

On Thu, Nov 15, 2018 at 11:13 AM Thomas Mortagne <thomas.mortagne@xwiki.com> wrote:

...

On Thu, Nov 15, 2018 at 11:06 AM Adel Atallah <adel.atallah@xwiki.com> wrote: > > Ok so it seems like we are getting back to the proposition we made with Vincent. > We need one annotation to enforce the dependence between parameters > (reference and type in our example) and another one that can be used > to *deduce* conflicting parameters.

> I don't understand how a hierarchy of groups can help us specify a > dependence between parameters.

I don't think it does, it's just that since we are defining groups having subgroups would be useful visually.

> A parameter is either in the same group > as another one or it is not. The hierarchy seems to focus on problems > that we are not trying to solve here. > The original proposal was similar to what Thomas proposed, but without > hierarchy: > > @Alternative("reference") > @Group("entityReference") > reference > > @Alternative("reference") > @Group("entityReference") > type > > @Alternative("reference") > page > > @Alternative("reference") > document > > where "Alternative" is the same as "Feature". Now Marius didn't agree > with that because the "Alternative" annotation should not be bind to > "reference" and "type" parameters but to the group "entityReference"

And as I said in my proposal the features are associated to the group, not the properties. I agree that associating it to the property (and ending up with half of a group conflicting with half of another) does really make sense.

This is not enforced by the code.

I don't understand what you mean, there is no code yet.

By code I meant the annotations in the code.

Yes but hard to do much better I think without breaking anything now that we have two parameters for a single information, we need to maintain them.

...

...

For me the code which is going to parse this Java bean will of course make sure the features are associated to the group of the property.

I agree with that.

...

...

You know that features are associated to groups *because* they are bound to the same property. Anyway I'm +1 to do it this way.

...

> , > which is not possible to do without creating other classes. I don't > think this is an issue to put the "Alternative" annotation on > "reference" and "type" because we should have all the necessary > information to *deduce* the conflicting parameters. It's true that > removing the "Alternative" annotation of one of "reference" or "type" > should produce the same result though, which could be confusing. > On Thu, Nov 15, 2018 at 10:23 AM Marius Dumitru Florea > <mariusdumitru.florea@xwiki.com> wrote: > > > > On Thu, Nov 15, 2018 at 10:51 AM Thomas Mortagne <thomas.mortagne@xwiki.com> > > wrote: > > > > > I'm also really not a fan of having to implement a component just to > > > indicate that two groups of properties are conflicting. > > > > > > +1 for making @Group support a hierarchy, that's indeed nice. > > > > > > For for conflicting we need a dedicated annotation IMO. > > > > > > So starting from your previous example I would expect something like: > > > > > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > > > > > > @PropertyGroup("target") > > > @PropertyFeature("reference") > > > page > > > > > > @PropertyGroup({"target", "entityReference"}) > > > @PropertyFeature("reference") > > > reference > > > > > > @PropertyGroup({"target", "entityReference"}) > > > type > > > > > > @PropertyGroup("target") > > > @PropertyFeature("reference") > > > document > > > > > > <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< > > > > > > > I don't think this is complete. The following doesn't make sense: > > > > {{include page="..." type="..."/}} > > > > and neither this: > > > > {{include document="..." type="..." /}} > > > > So it's not the reference parameter alone that provides the "reference" > > feature. The pair / group of parameters (reference and type) are providing > > the "reference" feature. This is why I think there is the need to specify > > the "feature" on the sub group "entityReference" not on the parameter. And > > to do this we need another class.. > > > > > > > > > > > or > > > > > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > > > > > > @PropertyGroup("target", features = "reference") > > > page > > > > > > @PropertyGroup({"target", "entityReference"}, features = "reference") > > > reference > > > > > > @PropertyGroup({"target", "entityReference"}) > > > type > > > > > > @PropertyGroup("target", features = "reference") > > > document > > > > > > <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< > > > > > > > > > > > * PropertyGroup define the hierarchy (also proposed a String[] instead > > > of String based value to show all possible ways to pass the hierarchy > > > value) > > > > > > > +1 for this > > > > > > > * PropertyFeature (name is negotiable :)) or PropertyGroup "features" > > > field associate the group with a set of unique "features". This is the > > > same logic than for extensions where several groups with with a shared > > > feature are in conflict > > > > > > > You're not associating the feature to the group. That is the problem IMO. > > You are associating the feature to the parameter. For instance: > > > > @PropertyGroup("foo", features = "input") > > one > > > > @PropertyGroup("foo", features = "output") > > two > > > > Is the "input" and "output" feature associate to the "foo" group or to the > > parameters one and two respectively? > > > > Thanks, > > Marius > > > > > > > > > > We could also decide to support only one feature per group right now > > > since we don't yet have the need for several but it felt more natural > > > like this. > > > > > > On Thu, Nov 15, 2018 at 8:04 AM Vincent Massol <vincent@massol.net> wrote: > > > > > > > > > > > > > > > > > On 15 Nov 2018, at 08:02, Vincent Massol <vincent@massol.net> wrote: > > > > > > > > > > > > > > > > > > > >> On 15 Nov 2018, at 06:29, Marius Dumitru Florea < > > > mariusdumitru.florea@xwiki.com> wrote: > > > > >> > > > > >> On Wed, Nov 14, 2018 at 5:12 PM Vincent Massol <vincent@massol.net> > > > wrote: > > > > >> > > > > >>> I thought about something like this but I discarded it as I find this > > > > >>> complicated for something that should be relatively simple. > > > > >> > > > > >> > > > > >> I don't think it's that complicated because: > > > > >> > > > > >> * Conflicting parameters should be an exception, not the rule. What > > > other > > > > >> macros, besides include / display, need this? > > > > >> * If you just want to group macro parameters for display then you > > > only need > > > > >> to use the @Group annotation. You don't need to implement a > > > ParameterGroup. > > > > >> The ParameterGroup is needed only for conflicting parameters (ATM). > > > > > > > > > > Sure but it’s still 10x more complicated than just having everything > > > in one place in the parameters class with annotations as was suggested > > > initially. > > > > > > > > And requires unnecessary component instances that will stay in the EM > > > for no need. The way to describe the descriptor is transient and only > > > serves to generate the macro descriptors. In the end what’s important is > > > the descriptor format. > > > > > > > > Thanks > > > > -Vincent > > > > > > > > > > > > > > Thanks > > > > > -Vincent > > > > > > > > > >> > > > > >> Thanks, > > > > >> Marius > > > > >> > > > > >> > > > > >>> I’d prefer to have some simple annotations if possible. In other > > > words, if > > > > >>> feels a bit of over-engineering for the need. Now I have to admit > > > that I > > > > >>> stopped following this thread after the original proposal so maybe > > > I’m just > > > > >>> completely off :) > > > > >>> > > > > >>> Thanks > > > > >>> -Vincent > > > > >>> > > > > >>>> On 14 Nov 2018, at 15:51, Marius Dumitru Florea < > > > > >>> mariusdumitru.florea@xwiki.com> wrote: > > > > >>>> > > > > >>>> WDYT about: > > > > >>>> > > > > >>>> -----8<----- IncludeMacroParameters ---------- > > > > >>>> @Group("target") > > > > >>>> page > > > > >>>> > > > > >>>> @Group("target/entityReference") > > > > >>>> reference > > > > >>>> > > > > >>>> @Group("target/entityReference") > > > > >>>> type > > > > >>>> > > > > >>>> @Group("target") > > > > >>>> document > > > > >>>> > > > > >>>> section > > > > >>>> > > > > >>>> context > > > > >>>> ----->8--------------- > > > > >>>> > > > > >>>> That is: specify *only* the group hierarchy in the macro parameter > > > > >>>> descriptor. This would produce the following hierarchy: > > > > >>>> > > > > >>>> * <target> > > > > >>>> ** page > > > > >>>> ** <entityReference> > > > > >>>> *** reference > > > > >>>> *** type > > > > >>>> ** document > > > > >>>> * section > > > > >>>> * context > > > > >>>> > > > > >>>> Next, for the cases where we want to customize the behavior of a > > > group, > > > > >>> we > > > > >>>> introduce a component role ParameterGroup. For instance, for the > > > "target" > > > > >>>> parameter group of the Include Macro we would create > > > > >>>> > > > > >>>> @Named("include/target") > > > > >>>> public class TargetParameterGroup implements ParameterGroup {} > > > > >>>> > > > > >>>> To specify that the members of a parameter group are exclusive we > > > can > > > > >>>> either use a method in the ParameterGroup interface (e.g. > > > isExclusive()) > > > > >>> or > > > > >>>> use an annotation on the implementation TargetParameterGroup. > > > > >>>> > > > > >>>> Thanks, > > > > >>>> Marius > > > > >>>> > > > > >>>> > > > > >>>> On Tue, Nov 13, 2018 at 12:03 PM Adel Atallah < > > > adel.atallah@xwiki.com> > > > > >>>> wrote: > > > > >>>> > > > > >>>>> Hello, > > > > >>>>> > > > > >>>>> I'd like to briefly summarize the situation so that we can make > > > some > > > > >>>>> progress. > > > > >>>>> > > > > >>>>> What we have: > > > > >>>>> * We define "parameters" in a macro by creating a Java Bean, which > > > > >>>>> provides all the getters and setters of the parameters we want. > > > > >>>>> * We can use annotations on these getters/setters to define some > > > > >>>>> behavior or metadata for these parameters (description, mandatory, > > > > >>>>> deprecated...) > > > > >>>>> > > > > >>>>> What we want: > > > > >>>>> * Being able to handle conflicting parameters. For instance when we > > > > >>>>> deprecate a parameter and add a new one to replace it, we should be > > > > >>>>> able to either use the deprecated parameter or the new one but not > > > > >>>>> both. > > > > >>>>> * We also want to group parameters that are related to each other. > > > > >>>>> > > > > >>>>> What we proposed: > > > > >>>>> * Use annotations on the parameters to express the conflict. > > > > >>>>> * Marius proposed to see the problem as a boolean expression such > > > as: > > > > >>>>> (page XOR (reference AND type) XOR document) OR section OR context. > > > > >>>>> This would translate as: the user can use the 'section' and/or > > > > >>>>> 'context' parameters (if they want), can use only one of these > > > > >>>>> parameters: 'page', ('reference' and 'type') or 'document', where > > > > >>>>> 'reference' and 'type' depend on each other and you can't use one > > > > >>>>> without the other. > > > > >>>>> * You can see on previous e-mails the kind of annotations we > > > proposed > > > > >>>>> to solve the issue. > > > > >>>>> > > > > >>>>> Thanks, > > > > >>>>> Adel > > > > > > > > > > > > > -- > > > Thomas Mortagne > > >

-- Thomas Mortagne

-- Thomas Mortagne

-- Thomas Mortagne

-- Thomas Mortagne