On Thu, Nov 15, 2018 at 10:23 AM Marius Dumitru Florea
<mariusdumitru.flo...@xwiki.com> wrote:
>
> On Thu, Nov 15, 2018 at 10:51 AM Thomas Mortagne <thomas.morta...@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 <vinc...@massol.net> wrote:
> > >
> > >
> > >
> > > > On 15 Nov 2018, at 08:02, Vincent Massol <vinc...@massol.net> wrote:
> > > >
> > > >
> > > >
> > > >> On 15 Nov 2018, at 06:29, Marius Dumitru Florea <
> > mariusdumitru.flo...@xwiki.com> wrote:
> > > >>
> > > >> On Wed, Nov 14, 2018 at 5:12 PM Vincent Massol <vinc...@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.flo...@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.atal...@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
