On Thu, Nov 15, 2018 at 11:13 AM Thomas Mortagne <[email protected]> wrote: > > On Thu, Nov 15, 2018 at 11:06 AM Adel Atallah <[email protected]> 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 > > <[email protected]> wrote: > > > > > > On Thu, Nov 15, 2018 at 10:51 AM Thomas Mortagne > > > <[email protected]> > > > 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 <[email protected]> > > > > wrote: > > > > > > > > > > > > > > > > > > > > > On 15 Nov 2018, at 08:02, Vincent Massol <[email protected]> wrote: > > > > > > > > > > > > > > > > > > > > > > > >> On 15 Nov 2018, at 06:29, Marius Dumitru Florea < > > > > [email protected]> wrote: > > > > > >> > > > > > >> On Wed, Nov 14, 2018 at 5:12 PM Vincent Massol <[email protected]> > > > > 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 < > > > > > >>> [email protected]> 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 < > > > > [email protected]> > > > > > >>>> 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
