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. > 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. > > 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. > 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. > > > 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
