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.

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.

  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.

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.

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







Reply via email to