On Fri, May 28, 2010 at 01:37, Richard S. Hall <he...@ungoverned.org> wrote:
> On 5/27/10 7:25 PM, Guillaume Nodet wrote:
>
>> On Fri, May 28, 2010 at 00:48, Richard S. Hall<he...@ungoverned.org>
>> wrote:
>>
>>
>>
>>> On 5/27/10 6:24 PM, Guillaume Nodet wrote:
>>>
>>>
>>>
>>>> Can those annotations be moved to a gogo specific package ? Something
>>>> like
>>>> org.apache.felix.gogo.annotation
>>>> I'd rather even have them in a separate module, but i think that would
>>>> cause
>>>> problem because the coercion mechanism do actually use those.
>>>>
>>>> I think those annotations are still experimental, not part of RFC 132.
>>>> There are still things that need to be fleshed out, and I'm really keen
>>>> on
>>>> implying that this will be 'the' way to create functions, until it can
>>>> support all the use cases.
>>>>
>>>>
>>>>
>>>>
>>> I think all of Gogo is experiment at this stage, thus the 0.x.0 version
>>> number. My guess is Gogo won't be able to go 1.0.0 until the RFC is
>>> complete, nor rejected and we just go our own way. So at this point, I
>>> don't
>>> see it as much of a concern. Clearly, once the OSGi packages stabilize,
>>> then
>>> we won't be able to mess with them.
>>>
>>> From an IP point of view, we can't include osgi packages that have not
>>> been
>>>
>>>
>> published yet.
>> I don't want to mess with such legal problems, but that's a fact. I had
>> to
>> remove some non published api from aries just because of that, and that's
>> also why i haven't modified the org.osgi.bundlerepository package when
>> working on OBR.
>> I think it's safer to do the same for gogo.
>>
>>
>
> I think this is really fuzzy. There has been no official publication of the
> org.osgi.service.command package by the OSGi Alliance at all, so that gets
> us to a slippery slope if that is our yardstick. The current packages were
> just contributed by Peter, but not as an act of the OSGi Alliance.
>
>
Well, it has been published in the following document:
http://www.osgi.org/download/osgi-4.2-early-draft2.pdf
And the license at http://www.osgi.org/Specifications/Licensing says:
"You are not authorized to create any derivative work of the Specification.
The OSGi Alliance also grants you a perpetual, non-exclusive, worldwide,
fully paid-up, royalty free, limited license (without the right to
sublicense) under any applicable copyrights, to create and/or distribute an
implementation of the Specification that: (i) fully implements the
Specification including all its required interfaces and functionality; (ii)
does not modify, subset, superset or otherwise extend the OSGi Name Space,
or include any public or protected packages, classes, Java interfaces,
fields or methods within the OSGi Name Space other than those required and
authorized by the Specification."
I agree it's a pain, especially when we want to prototype / enhance as it is
the case here. As I said, I have the exact same problems with subsystems
in aries.
IANAL, but I'd rather stay away from those problems.
>
>>
>>> However, I'm not against moving all packages inside of
>>> org.apache.felix.gogo package space and just avoid using org.osgi package
>>> space altogether, but I think this is extreme for a module that hasn't
>>> yet
>>> undergone a major release yet.
>>>
>>> I don't think we need to move all. The existing packages have been
>>> proved
>>>
>>>
>> to be very usable and the annotations are quite untied so far, so I don't
>> see why moving everything would be needed. It would be rather disruptive
>> without no value I think.
>>
>>
>
> See above. I don't see how we can justify a partial approach if there is a
> real IP issue here. Although, from some discussion on the other Apache
> mailing lists, it seems that there is some debate as to whether using
> someone else's package space infringes on their IP.
Yeah, not really sure what you're referring to exactly. I personally think
we should avoid modifying packages that do not start with org.apache.
>
>
> I think one very important use case is support for completion of
>>> arguments
>>>
>>>
>>>> (not only command names), which i'm not sure how it will fit yet.
>>>>
>>>>
>>>>
>>>
>>>
>> The inspect commands looks really cool. That's one that would really
>> benefit from completion btw. I wonder if using enums instead of strings
>> for the type and direction parameters would help here.
>>
>>
>
> Yeah, completion would be nice. Not sure enums would make a difference or
> not. If you think it helps, feel free to suggest a patch.
Well, I guess the problem is that what we have in Karaf does not fit at all.
We extend the Function interface to allow the retrieval of completers:
http://svn.apache.org/repos/asf/felix/trunk/karaf/shell/console/src/main/java/org/apache/felix/karaf/shell/console/CompletableFunction.java
The problem is that annotations are too static to be leveraged for
completers. Being able to specify only a class name might be a bit too
simplistic as some completers need to access some osgi services or other
kind of resources.
Anyway, I don't want to waste too much time on rewriting something I'm not
sure to use yet, so don't really expect a patch here ...
>
>
> In addition having string descriptions inside the code might be
>>> problematic
>>>
>>>
>>>> for localization, and the annotations for descriptions are still a bit
>>>> fuzzy
>>>> as there is a @Descriptor, but descrition attributes on @Flag and
>>>> @Option
>>>> which looks redundant or misleading from an api pov.
>>>>
>>>>
>>>>
>>>>
>>> Yeah, we still need to think about localization.
>>>
>>> Regarding the redundant description, it is true it is redundant, and I
>>> experimented today with having the description only in Descriptor, but it
>>> was much more cumbersome to work with that way, so I put it back. If
>>> annotations supported interface extension we wouldn't have this issue at
>>> all.
>>>
>>>
>>
>> Yeah, and I actually missed you latest commits.
>>
>> I think it may be a bit cleaner to remove the description attribute on the
>> @Parameter. It might be a bit more verbose in some cases, but much more
>> consistent I think.
>>
>>
>
> Perhaps. The issue wasn't the verbosity, it was the handling. For example,
> in the help system when handling the annotations I'd have to get the
> @Parameter annotation, which contained all of the information about the
> parameter except the description, which forced me to have to get the
> @Descriptor annotation too. This ultimately made it more difficult to do
> things like dropping it in a Map<String, Parameter> to keep track of
> parameters by name, since I also need to associate the description somehow,
> forcing me to keep another Map<String, Descriptor>... To me, that seems like
> bad API design too.
>
The fact that the api provided by the JRE to access annotations is far from
easily usable should not reflect on the API design I think. That's mostly
an implementation detail which should be hidden to the user.
The problem is that now you have two ways to provide the description.
Also a way to handle the localization problem may be something like:
@Descriptors({
@Descriptor("default"),
@Descriptor("localized", locale = "en_US")
})
@Parameter( ... )
This example may not compile as is, but there is a way to do something like
that. So keeping the description on a dedicated annotation may be a good
idea.
>
> If only annotations didn't suck so hard...
>
> -> richard
>
>
>
>>
>>>
>>> I just want to make my point clear before a release is cut ;-)
>>>
>>>
>>>>
>>>>
>>>>
>>> Thanks.
>>>
>>> -> richard
>>>
>>>
>>>
>>>
>>>
>>>> On Wed, May 5, 2010 at 07:55, Peter Kriens<peter.kri...@aqute.biz>
>>>> wrote:
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>> I guess we should have tried this out in a sandbox but this experiment
>>>>> seems rather harmless for existing users and potentially very powerful.
>>>>> And
>>>>> this experiment is timely because I do have to update RFC 147. Will do
>>>>> different next time :-)
>>>>>
>>>>> On 4 mei 2010, at 08:33, Guillaume Nodet wrote:
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> A few things that would be missing imho to make that interesting:
>>>>>> * parameters annotation to mark parameters as optional or
>>>>>> multi-valued
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>> How do you see this? We do have the type of the parameter so we can
>>>>> deduce
>>>>> it is multi value. However, how do we know the last one? We would need
>>>>> some
>>>>> kind of separator. And do not forget that we have real objects, not
>>>>> strings.
>>>>>
>>>>> We had not gotten around the optional mandatory part. I think I'd like
>>>>> to
>>>>> have a special annotation for that.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> * flag and option could be merged (they are the same, maybe use an
>>>>>> enum
>>>>>> value on the annotation to differentiate them and maybe have a smart
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>> default
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> value based on the fact that the annotated parameter is a boolean or
>>>>>> not)
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>> Well, it is shorter and easier on the eyes to have different
>>>>> annotations
>>>>> I
>>>>> think.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> * option should support a list of value
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>> Not sure how this interplays with the type we also have.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> * options should have multiple names (annotations support String[]
>>>>>> quite
>>>>>> well)
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>> Ok.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> * if we want to add some help information, we need an annotation on
>>>>>> the
>>>>>> method
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>> Yes. I think some help on the arguments would also be nice.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> * we have a nice coercion mechanism defined in blueprint (handling
>>>>>> generics, collections and such), we should reuse it (without having a
>>>>>> dependency to blueprint)
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>> I agree we should follow the same rules in tsh though implementation
>>>>> code
>>>>> will differ because the args/options/flags will have to be parsed in
>>>>> the
>>>>> heart of the method selection algorithm, they directly interfere with
>>>>> it.
>>>>> The low level type converter should be very similar to the BP type
>>>>> converter
>>>>> I think. When working on the BP converter I used a lot of lessons from
>>>>> tsh.
>>>>> However, the BP type converter takes generics better into account.
>>>>>
>>>>> Kind regards,
>>>>>
>>>>> Peter Kriens
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> On Mon, May 3, 2010 at 23:25, Richard S. Hall<he...@ungoverned.org>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>> wrote:
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>> On 5/3/10 16:56, Guillaume Nodet wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>> On Mon, May 3, 2010 at 21:17, Richard S. Hall<he...@ungoverned.org>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>> On 5/3/10 14:51, Guillaume Nodet wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> What are those annotations suppose to actually provide ? It seems
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>> all
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>
>>>>>
>>>>>> they
>>>>>>
>>>>>>
>>>>>>> can do is provide some basic help to the user, but does not really
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>> help
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>
>>>>>
>>>>>> the
>>>>>>
>>>>>>
>>>>>>> user writing complex commands and dealing with complex arguments.
>>>>>>>>>>
>>>>>>>>>> Have a look at an existing example:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>> http://svn.apache.org/repos/asf/felix/trunk/karaf/shell/commands/src/main/java/org/apache/felix/karaf/shell/commands/GrepAction.java
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> I think those annotations would not provide the slightest help in
>>>>>>
>>>>>>
>>>>>>> analyzing
>>>>>>>>>> such a command line.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>> I don't think there will ever be a single solution that can help
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>> everyone
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>
>>>>>
>>>>>> implement any possible command. If people need to do something that is
>>>>>>
>>>>>>
>>>>>>> super
>>>>>>>>> complex, then they can always fall back to parsing their own
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>> arguments.
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>> I can agree to that, but then I don't see why it has to be in the
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>> proposed
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>>> spec. Having it as a separate module might be a better approach then.
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>> The issue is you cannot easily do what we are trying to achieve in a
>>>>>>> separate module since it is modifying how the runtime coerces
>>>>>>> arguments
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>> when
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> invoking methods. There is no place to hook into this externally.
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>> However, I
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> agree that it could be just an implementation-specific feature of the
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>> Gogo
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> runtime.
>>>>>>
>>>>>>
>>>>>>> I actually talked with Peter specifically about this and he felt the
>>>>>>> feature might be worthwhile for the spec, which is why he committed
>>>>>>> it
>>>>>>> there, but nothing is set in stone at this point.
>>>>>>>
>>>>>>> -> richard
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>> At this point, we're just messing around with allowing optional
>>>>>>>>> method
>>>>>>>>> parameters and out of order specification of arguments.
>>>>>>>>>
>>>>>>>>> -> richard
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Mon, May 3, 2010 at 19:01,<pkri...@apache.org> wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> Author: pkriens
>>>>>>>>>>> Date: Mon May 3 17:01:53 2010
>>>>>>>>>>> New Revision: 940514
>>>>>>>>>>>
>>>>>>>>>>> URL: http://svn.apache.org/viewvc?rev=940514&view=rev
>>>>>>>>>>> Log:
>>>>>>>>>>> Annotations for parameters
>>>>>>>>>>>
>>>>>>>>>>> Added:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>> felix/trunk/gogo/runtime/src/main/java/org/osgi/service/command/Flag.java
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>> felix/trunk/gogo/runtime/src/main/java/org/osgi/service/command/Option.java
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> Added:
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>> felix/trunk/gogo/runtime/src/main/java/org/osgi/service/command/Flag.java
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> URL:
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>> http://svn.apache.org/viewvc/felix/trunk/gogo/runtime/src/main/java/org/osgi/service/command/Flag.java?rev=940514&view=auto
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>> ==============================================================================
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> ---
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>> felix/trunk/gogo/runtime/src/main/java/org/osgi/service/command/Flag.java
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> (added)
>>>>>>
>>>>>>
>>>>>>> +++
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>> felix/trunk/gogo/runtime/src/main/java/org/osgi/service/command/Flag.java
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> Mon May 3 17:01:53 2010
>>>>>>
>>>>>>
>>>>>>> @@ -0,0 +1,6 @@
>>>>>>>>>>> +package org.osgi.service.command;
>>>>>>>>>>> +
>>>>>>>>>>> +public @interface Flag {
>>>>>>>>>>> + String name();
>>>>>>>>>>> + String help() default "no help";
>>>>>>>>>>> +}
>>>>>>>>>>>
>>>>>>>>>>> Added:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>> felix/trunk/gogo/runtime/src/main/java/org/osgi/service/command/Option.java
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> URL:
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>> http://svn.apache.org/viewvc/felix/trunk/gogo/runtime/src/main/java/org/osgi/service/command/Option.java?rev=940514&view=auto
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>> ==============================================================================
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> ---
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>> felix/trunk/gogo/runtime/src/main/java/org/osgi/service/command/Option.java
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> (added)
>>>>>>
>>>>>>
>>>>>>> +++
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>> felix/trunk/gogo/runtime/src/main/java/org/osgi/service/command/Option.java
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> Mon May 3 17:01:53 2010
>>>>>>
>>>>>>
>>>>>>> @@ -0,0 +1,6 @@
>>>>>>>>>>> +package org.osgi.service.command;
>>>>>>>>>>> +
>>>>>>>>>>> +public @interface Option {
>>>>>>>>>>> + String name();
>>>>>>>>>>> + String dflt();
>>>>>>>>>>> +}
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>> --
>>>>>> Cheers,
>>>>>> Guillaume Nodet
>>>>>> ------------------------
>>>>>> Blog: http://gnodet.blogspot.com/
>>>>>> ------------------------
>>>>>> Open Source SOA
>>>>>> http://fusesource.com
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>
>>
>>
>
--
Cheers,
Guillaume Nodet
------------------------
Blog: http://gnodet.blogspot.com/
------------------------
Open Source SOA
http://fusesource.com
