-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
support batchee cli #20
Comments
+1 |
Here what we need to support: [source]
As you can see we need option with a single dash but a word behind and not double dashes |
Airlift supports a single dash?? I'm very surprised by that. On Wed, Aug 27, 2014 at 2:36 AM, Romain Manni-Bucau <
|
I see. Airline allows you to hang yourself. On the surface looks like it would support "+++foo=bar" if you wanted. We're definitely doing that. I'll see if I can hack in some sort of non-intrusive compatibility. David Blevins On Aug 27, 2014, at 11:13 AM, David Blevins [email protected] wrote:
|
I didn't understand. |
https://github.com/airlift/airline Written by friends. Slightly different style for the annotations. Great features, but makes you create lots of command objects. I prefer JAX-RS style to consuming parameters rather than Servlet "one big object with everything" style. You spend time pulling the data back out. On Aug 27, 2014, at 12:23 PM, "Daniel Cunha (soro)" [email protected] wrote:
|
Well only issue of airline is it brings the world to work Adding automatically - or -- in crest was an error imo (think i already said it): restrict what you can do. Other point is -foo=bar versus -foo bar. Last feature helping here: class bean parameter injection (for jaxrs config). It makes at least 3 features ;). If one doesnt match crest goal please let me know ill do something easier for batchee. |
Single dash for long parameters is fine, it just means those commands will not be able to support Posix type actions like: tar -xzvf ... Posix will remain the default and those commands will not require additional steps. Additional config or annotations would be required for commands that want to deviate. Supporting space instead of '=' would be nice. Just will take some work. David Blevins On Aug 27, 2014, at 1:31 PM, Romain Manni-Bucau [email protected] wrote:
|
isn't it easy? if first char not a alphanum then don't prefix :) |
If you're referring to '--foo bar' being the same as '--foo=bar', go for it. Just make sure there are several tests and a couple that try and create some confusing scenarios. When you dig into the code you'll see it's not quite as trivial as you imagined. Currently, all parameters have values. When not specified we default. We haven't released 1.0 yet, so we can change some of the behavior which we might have to do. David Blevins On Aug 27, 2014, at 10:06 PM, Romain Manni-Bucau [email protected] wrote:
|
pushed just prefix handling: if option already starts with - then keep it this way (easy to support + later if needed) for =/space that's harder cause you need to count what's after to see what the user did |
Add more tests. The "--no-" logic was updated, so that should be tested. Also help is not tested for this new style. We should also test what happens if short prefixes are used in the same command as long -- should Posix stringing be supported which relies on "-" vs "--"? Should we allow "----foo" to be different/same as "---foo" as "--foo" ? David Blevins On Aug 27, 2014, at 10:57 PM, Romain Manni-Bucau [email protected] wrote:
|
help is tested no is not since it will not respect the style (at least one - more) so that's a not important stuff for now in my tes -value != ----value and would be != --value we have to support short in long prefixes for List at least willl try to go ahead later with all requirements PS: what about:
|
I'd say let's not support "------foo". There is such a thing as ruing a For now let's stick to just '-foo", no infinite dashes or any other prefix. On #2, this should be supported as of Jon's earlier commit. On Wed, Aug 27, 2014 at 11:39 PM, Romain Manni-Bucau <
|
Hmm ok so surely better to revert my commit. I'll go with something custom in batchee |
That's fine too. I'm sure it wouldn't be that hard to support "-foo" in crest cleanly and consider the edge cases. Not 15 minutes, but not 2 weeks either. The majority of Crest and the 150~ tests were done in 4 days. David Blevins On Aug 28, 2014, at 12:13 AM, Romain Manni-Bucau [email protected] wrote:
|
Well on my side issue is bigger since +xxx is a good cli API (think to chmod) but not more blocking you can close this issue then. |
That's a really good point. Hadn't considered chmod style commands. Looking at the chmod style, it also appears collapsable, like posix short dash options. So for example, these two are equivalent: chmod +g=x,+o=x foo.txt It also appears chmod supports still the posix single dash collapsing, so these two are the same: chmod -fhv go=x foo.txt As I keep saying, any functionality is good provided it has value, is well thought out and is thoroughly tested. This is a good discussion, so no reason to close the issue so quickly. David Blevins On Aug 28, 2014, at 1:18 AM, Romain Manni-Bucau [email protected] wrote:
|
means implementing in java http://code.woboq.org/userspace/glibc/posix/getopt.c.html but the space/equal separator stays an issue. That said if we parse the line char by char (so it means we need a parser for a dynamic grammar, yeah ;)) we can handle it if that's a parameter of @option |
PS: why not backing our impl with [cli]? Just doing a annotation layer on top of it? |
using crest instead of airline for batchee would help removing a bunch of undesired dependencies but today it can't replace it 100% AFAIK, would be great to support it
here is the module https://git-wip-us.apache.org/repos/asf?p=incubator-batchee.git;a=tree;f=tools/cli;h=99ec46928e0edf824d2a9d8e3a3f7461185331b2;hb=e62c3ee54cb448e9d8086c7a2940523b0f0332a7
The text was updated successfully, but these errors were encountered: