15 Nov
2018
15 Nov
'18
3:13 p.m.
On Thu, Nov 15, 2018 at 2:06 PM Adel Atallah <adel.atallah(a)xwiki.com> wrote:
--
Thomas Mortagne
On Thu, Nov 15, 2018 at 1:56 PM Thomas Mortagne
<thomas.mortagne(a)xwiki.com> wrote:
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.
On Thu, Nov 15, 2018 at 1:24 PM Adel Atallah <adel.atallah(a)xwiki.com> wrote:
By code I meant the annotations in the code.
On Thu, Nov 15, 2018 at 11:13 AM Thomas Mortagne
<thomas.mortagne(a)xwiki.com> wrote:
I don't understand what you mean, there is no code yet.
On Thu, Nov 15, 2018 at 11:06 AM Adel Atallah <adel.atallah(a)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. 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(a)xwiki.com> wrote:
> > > > >
> > > > > On Thu, Nov 15, 2018 at 10:51 AM Thomas Mortagne
<thomas.mortagne(a)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(a)massol.net> wrote:
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > > On 15 Nov 2018, at 08:02, Vincent Massol
<vincent(a)massol.net> wrote:
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > >> On 15 Nov 2018, at 06:29, Marius Dumitru
Florea <
> > > > > > mariusdumitru.florea(a)xwiki.com> wrote:
> > > > > > > >>
> > > > > > > >> On Wed, Nov 14, 2018 at 5:12 PM Vincent
Massol <vincent(a)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(a)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(a)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