Re: [xwiki-devs] [Need proposal] How to show "conflicting" macro parameters

On 16 Nov 2018, at 10:41, Adel Atallah &lt;adel.atallah(a)xwiki.com&gt; 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 &lt;thomas.mortagne(a)xwiki.com&gt; wrote: > > On Thu, Nov 15, 2018 at 2:06 PM Adel Atallah &lt;adel.atallah(a)xwiki.com&gt; wrote: >> >> On Thu, Nov 15, 2018 at 1:56 PM Thomas Mortagne >> &lt;thomas.mortagne(a)xwiki.com&gt; wrote: >>> >>> On Thu, Nov 15, 2018 at 1:24 PM Adel Atallah &lt;adel.atallah(a)xwiki.com&gt; wrote: >>>> >>>> On Thu, Nov 15, 2018 at 11:13 AM Thomas Mortagne >>>> &lt;thomas.mortagne(a)xwiki.com&gt; wrote: >>>>> >>>>> On Thu, Nov 15, 2018 at 11:06 AM Adel Atallah &lt;adel.atallah(a)xwiki.com&gt; 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 >>>>>> &lt;mariusdumitru.florea(a)xwiki.com&gt; wrote: >>>>>>> >>>>>>> On Thu, Nov 15, 2018 at 10:51 AM Thomas Mortagne &lt;thomas.mortagne(a)xwiki.com&gt; >>>>>>> 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 &lt;vincent(a)massol.net&gt; wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>>> On 15 Nov 2018, at 08:02, Vincent Massol &lt;vincent(a)massol.net&gt; wrote: >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>>> On 15 Nov 2018, at 06:29, Marius Dumitru Florea < >>>>>>>> mariusdumitru.florea(a)xwiki.com&gt; wrote: >>>>>>>>>>> >>>>>>>>>>> On Wed, Nov 14, 2018 at 5:12 PM Vincent Massol &lt;vincent(a)massol.net&gt; >>>>>>>> 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(a)xwiki.com&gt; 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(a)xwiki.com&gt; >>>>>>>>>>>>> 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