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)
* 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