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 <[email protected]> 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
> <[email protected]> wrote:
> >
> > On Thu, Nov 15, 2018 at 2:06 PM Adel Atallah <[email protected]> wrote:
> > >
> > > On Thu, Nov 15, 2018 at 1:56 PM Thomas Mortagne
> > > <[email protected]> wrote:
> > > >
> > > > On Thu, Nov 15, 2018 at 1:24 PM Adel Atallah <[email protected]>
> > > > wrote:
> > > > >
> > > > > 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.
> > > >
> > > > 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
> > > > > > > <[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
> > > >
> > > >
> > > >
> > > > --
> > > > Thomas Mortagne
> >
> >
> >
> > --
> > Thomas Mortagne
--
Thomas Mortagne
