This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 57611

Summary: Command-line interface (CLI) API
Product: platform Reporter: Jan Chalupa <jchalupa>
Component: -- Other --Assignee: Jaroslav Tulach <jtulach>
Status: RESOLVED FIXED    
Severity: blocker CC: anebuzelsky, jglick, jlahoda, mpetras, pnejedly
Priority: P3 Keywords: API_REVIEW
Version: 5.x   
Hardware: All   
OS: All   
Issue Type: ENHANCEMENT Exception Reporter:
Bug Depends on: 81195, 84800, 84802, 84803, 84804    
Bug Blocks: 57364, 69078    
Attachments: Changes in core and utilities to make use of the sendopts module
New javadoc for the work being done on redesign_57611 of contrib/sendopts
SendOpts API simplified for easy tasks

Description Jan Chalupa 2005-04-07 14:02:10 UTC
The platform should provide an API to command-line interface (CLI) so that
modules can easily extend it and add command-line options they want to support.

In addition, the API should provide ability to handle arguments (e.g. list of
files) that are not prefixed with a specific option. For example, it should be
possible to append a list of files to be opened by the IDE to the command-line
and have the utilities module to handle it without having to specify the '--open
<file>' option. See issue #57364 for more info on this use case.
Comment 1 Jaroslav Tulach 2005-09-02 21:47:52 UTC
Created attachment 24487 [details]
Changes in core and utilities to make use of the sendopts module
Comment 2 Jaroslav Tulach 2005-09-02 21:50:08 UTC
Basic API is in contrib/sendopts and the changes needed in core&utilities are 
attached. Probably ready for next version (4.1 + 2). 
 
Btw. parts of the patch have to be merged for 4.2, as I found out that 
CLIOptions2 in core/src are never called. The patch here also fixes that. 
Comment 3 Jaroslav Tulach 2006-07-24 16:14:03 UTC
I'd like to get feedback on the "sendopts" API which is capable of fixing the 
problems that initiated this issue. First of all it provides a reasonably well 
justified, designed and stable API and also it enhances the possible features, 
so one can write a handler for list of files prefixed by no command line 
option.

jglick, pnejedly, jlahoda agreed to look at the API and provide initial 
comments. Hopefully mpetras (as owner of utilities that provide --open) will 
have time for review as well and of course feedback of others is welcomed as 
well.

This is supposed to be inception review to help to scope and setup goals. That 
is why the main review material should be the list of usecases and other 
architecture questions available here:
http://www.netbeans.org/download/dev/javadoc/org-netbeans-modules-sendopts/architecture-summary.html
The goal is to check that the usecases are complete and describe the problems. 
Of course you can check the whole javadoc and try the actual implementation, 
but only if you agree that the usecases are valid:
http://www.netbeans.org/download/dev/javadoc/org-netbeans-modules-sendopts/overview-summary.html

Thanks for you help with the inception review. It is appreciated as it will 
serve as an excellent feedback for planning of 6.0 and subsequent releases.
Comment 4 Jan Lahoda 2006-07-26 18:18:05 UTC
Seems quite good to me. I have a few small comments though.

Regarding the usecases:
JL1: imagine that I have two options: "--folder <path>" and "--file
<file-name-relative-to-folder>". Imagine that --folder is optional, defaulting
to Env.getCurrentDirectory(). Imagine that the user is allowed to provide the
--folder and --file options in any order - currently this seems to be quite hard
to implement. Maybe the correct way is Option.pair(Option.defaultOneOf(null,
folderOption), fileOption, <some-processor>), but I did not find any usecase
that would cover Option.defaultOneOf and the method does not have a javadoc, so
I cannot be sure. I think this might be a usefull usecase. Any opinions?

JL2: imagine I have "-open-file" option, with an argument. Currently, if I
assign a help to it, it gets printed as:
-open-file <arg> /the help text/
I think it would be usefull if I could change the "<arg>" into something more
appropriate, like "<file>".

JL3: I do not fully understand the reason for usecase "Just Parse the Command
Line" - who is going to call this in reality? Why do we support this usecase?
(This is not to say that I am against this usecase, it is only to be sure we
really want to support it.)

Others:
JL4: Env uses Streams (PrintStream and InputStream) - using Writers/Readers may
be more appropriate, as the server and client may run on different locales with
different file encodings.

JL5: Help performance: issue #81195.

JL6: Please document required format of the bundle name in Option.shortDescription.
Comment 5 Jesse Glick 2006-07-27 13:08:39 UTC
[jglick01] Probably the module could be autoload.
Comment 6 Jesse Glick 2006-07-27 13:14:03 UTC
[jglick02] I am unclear why there is a separate source root clisrc. It is built
into the main module JAR anyway. Putting ${cp.cli} into project.properties
simply obscures the (important) dependency on boot.jar. Unless there is some
compelling reason why these sources need to be kept in a separate root I would
suggest putting them in with the regular module sources and simplifying the
project structure. This would also permit Handler and HandlerImpl to be merged,
making the code more comprehensible.
Comment 7 Petr Nejedly 2006-07-27 16:01:42 UTC
PN1: There's no CommandException.exitCode(String). By the availabe factories,
you're forcing API users to do I18N and do it efficiently*, but they might get
(already localized) error String from some other subcomponent, w/o knowledge of
the defining bundle and the key.

*) not really. The act of retrieving (=parsing) bundle is mostly going to be
expensive enough to eclipse the time spared by not performing the key lookup and
message formatting.

PN2: It took me a while to understand why the options are typed and why there is
a return value from the processor at all. Wouldn't the API be much simpler to
understand if there was no type, no return value, and only Strings are passed
internally. Most SPI usages would end up using "Void" as the type argument
anyway, returning null from the processors.

PN3: As you'll probably want to provide short description for every option,
using Option.shortDescription wrapper looks clumsy. Generally, using {String
bundle, String key} for localization is quite uncommon pattern.

Comment 8 Jaroslav Tulach 2006-07-27 16:20:23 UTC
JL1, I'll describe the usecase. Is JL2 meant to be tcr or tca? 
JL3: The API was intended to be NetBeans independent as much as possible and 
potentially useful in standalone applications. They should then use methods in 
the CommandLine interface. [jglick02] This is also the reason why the clisrc 
is separated. I wanted the module to run and possibly compile independently on 
NetBeans (except openide/util). But I do not think this is needed anymore, 
I'll merge the two source roots to src while moving the module from contrib to 
core/sendopts (btw. is that acceptable location?).

JL4: True about the different locale. Actually I wanted to add Locale 
Env.getLocale(), adding Reader and Writer is also possible (choose whether it 
is a TCA or TCR), but Input and Output Streams need to stay - the 
http://dvbcentral.sf.net streams mpeg's thru the stdout and the file might get 
corrupted if this was done thru Writer.

JL6: I agree that a lot of documentation is missing, I did not want to invest 
more time to that unless it is clear that we like the API and want to include 
it in the platform.

[jglick01] I made the module autoload. PN1 and PN3, I can fix it, just tell me 
how. PN2: True that most of the implementations will use Void as return type, 
but there is nothing wrong on that I guess. As far as I know it is a common 
pattern when writing generics, see for example 
http://download.java.net/jdk6/docs/api/javax/lang/model/element/ElementVisitor.html
Comment 9 Jan Lahoda 2006-07-28 08:46:36 UTC
JL2&JL4: probably TCA's, if others agree.
Comment 10 Petr Nejedly 2006-07-28 08:53:10 UTC
PN1a: CommandException.exitCode(int code, String localizedMessage)
Unless you expect to do a cross-locale translation (server running en_US, client
running cs_CZ, infrastructure translating server-originated exception to client
locale. But that won't work with ResourceBundle argument anyway.

PN2a: It's unpleasant to needlessly return null from a "Void" method. Anyway, at
least document the type parameter and its most common usage ("Void", "return
null") prominently (like the mentioned ElementVisitor).

Comment 11 Jesse Glick 2006-07-28 08:59:01 UTC
core/sendopts is I guess OK though I do not like the practice of putting
API-providing modules into core/*.


[jglick03] I am struggling to understand the sections "Complex Option Relations"
and "Alternative Options". Looks rather complex to me and I cannot think of any
NB code which would need it. Can these things be dropped? Surely it is best for
the processor itself to handle any complicated logic relating to sets of
arguments. I am concerned that the API is too large compared to the relatively
simple things we really need to do with it.

Also, Option.pair only works on two related options? Not three or more?

Perhaps the reason you thought it necessary to have these complex composite
option APIs is that the way OptionProvider works is IMHO unnatural. I would
expect to see e.g.

Option[] getOptions();
void handle(OptionInstance[]);

where

struct OptionInstance {
  Option option; // one of getOptions()
  Env env;
  String[] args; // may be empty, 1, even >1 for addnl args
}

This way I can see all the options that I claim to handle at once and decide for
myself what to do with them, in which order.


[jglick04] Prefer to remove CommandLine.getDefault() and make methods static, if
there is no intended semantics for a CommandLine instance.


[jglick05] CommandLine.parse Javadoc doesn't answer my question: what does this
method do? It "parses" arguments but returns void. So...? Does it invoke
handlers or not?


[jglick06] Many methods in Option are undocumented.


[jglick07] What is the purpose of Option.optionalArgument with no processor?


[jglick08] I am as mystified as Petr by the generic type signatures. Makes API
harder to understand and does not seem to add anything. Option.process in
particular is too much to read. Also all type params need Javadoc. (use @param
in class or method Javadoc)
Comment 12 Jaroslav Tulach 2006-08-04 14:37:51 UTC
Created attachment 32541 [details]
New javadoc for the work being done on redesign_57611 of contrib/sendopts
Comment 13 Jaroslav Tulach 2006-08-04 14:43:30 UTC
Reviewers, please see the attached JavaDoc. I have implement all your 
requirements, as you can verify in the current version of the opinion 
document:
http://www.netbeans.org/source/browse/*checkout*/openide/www/tutorial/reviews/opinions_57611.html?rev=1.3
and I'd like to start new round of review. Please look at the javadoc and tell 
me if it is acceptable. If so, I will provide patch for utilities/openfile 
(needs to use the API) and core/bootstrap (needs another friend) and then I'd 
like to integrate. Please comment by Friday 2006/08/11.
Comment 14 Jesse Glick 2006-08-04 17:18:19 UTC
I thought we had agreed that "composite" option types were unnecessary if an
option processor can be given all options at once. Yet Option.*Of methods
remain, and worse, they are mandatory due to the OptionProcessor constructor.
This style still looks unnecessarily complex and confusing to me. I would
recommend simplifying it as follows:

1. OptionProcessor to take Option[] in its constructor, representing all options
it understands.

2. Option.{one,some,all}Of methods to be deleted.

OptionProcessor implementations are anyway free to perform any kind of
validation they like of their options and arguments, including but not limited
to logic relating to the combinations of different options, possibly sensitive
to actual argument values. Hardcoding this kind of logic into the API
accomplishes nothing that I can see.


CommandException factory methods look strange, especially as they are all named
exitCode even though that is just one parameter (unhelpfully named 'e'). Prefer
regular constructors with meaningful parameter names as is customary and
expected for Exception subclasses. There is no compelling reason to use factory
methods to construct a final class with no apparent chance of substitution in
the future.


Other things discussed at the meeting are not implemented, not sure if you still
plan them or not:

1. Use of char rather than int for short names.

2. Removal of Option.shortDescription method and replacement with optional
String shortDescription parameter to existing factory methods.


Need clearer statement of what an option short description is used for. Is the
line of text printed verbatim? Is the option name prepended? Is anything else
printed by the infrastructure? Prefer for the infrastructure not to interfere
with the help text too much, since the author of the option probably knows best
how to present it, e.g.

  --open <file1> <file2...>      Open the named files.

Just need some guidelines for conventional formatting.
Comment 15 Jaroslav Tulach 2006-08-25 16:27:20 UTC
Created attachment 33282 [details]
SendOpts API simplified for easy tasks
Comment 16 Jaroslav Tulach 2006-08-25 16:49:31 UTC
Reviewers here my new version that I'd like to be reviewed by Wed, Sep 13. On 
that day I'd like to have a final vote. 

JL1: Just use OptionProcessor and return two options from the getOptions 
method, get their values and process them as you wish

JL2: Use Option.displayName

JL3: External applications if they wish to use the API outside of NetBeans

JL4: Streams are still there, I need them to transfer binary data. No locale 
support yet, but can be added compatibly when needed.

JL5: done

JL6: Anything still left to do? Please tell me.

jglick01: Made autoload

jglick02: Sources moved to one src root

PN1: Now there is a way to construct CommandException with just a localized 
string

PN2: Simplified.

PN3: After discussion with reviewers it has been agreed that the lazy binding 
with locale is needed:
http://www.netbeans.org/servlets/ReadMsg?list=nbdev&msgNo=35329

PN1a: Removed.

PN2a: Removed.

jglick03: Done, OptionProcessor takes Map

jglick04: Not done, I still expect one might want more than one configuration 
of provides in the system and then the nonstatic methods will be handy.

jglick05: Name changed and documented.

jglick06: Documented.

jglick07: No longer there.

jglick08: Option class is now without generics and is simple to use.

Plus answers in:
http://www.netbeans.org/source/browse/*checkout*/openide/www/tutorial/reviews/opinions_57611.html?rev=1.4

Re. removal of xyzOf:
http://www.netbeans.org/servlets/ReadMsg?list=nbdev&msgNo=35379

CommandException.exitCode can be transferred to constructor before 
integration, if it is the last problem. 


Comment 17 Jesse Glick 2006-09-13 14:42:48 UTC
Approved, so Yarda will for M4:

- implement TCRs/TCAs listed as blockers

- increment module name to /2

- place in core/sendopts (can probably delete copy in contrib/sendopts)

- use in utilities, scripting, and performance/insane modules

- fix up core/bootstrap/nbproject/project.xml to not list utilities as a friend
Comment 18 Jaroslav Tulach 2006-09-18 17:57:47 UTC
Thanks, I am working on it.
Comment 19 Jaroslav Tulach 2006-09-18 18:18:37 UTC
Fixed all, except rewrite of performance/insane.

IDE:-------------------------------------------------
IDE: [18.9.06 19:15] Committing started
Checking in ide/golden/public-packages.txt;
/shared/data/ccvs/repository/ide/golden/public-packages.txt,v  <--  
public-packages.txt
new revision: 1.58; previous revision: 1.57
done
Checking in ide/golden/files-layout.txt;
/shared/data/ccvs/repository/ide/golden/files-layout.txt,v  <--  
files-layout.txt
new revision: 1.151; previous revision: 1.150
done
Checking in ide/golden/modules.txt;
/shared/data/ccvs/repository/ide/golden/modules.txt,v  <--  modules.txt
new revision: 1.68; previous revision: 1.67
done
Checking in ide/golden/deps.txt;
/shared/data/ccvs/repository/ide/golden/deps.txt,v  <--  deps.txt
new revision: 1.304; previous revision: 1.303
done
Checking in ide/golden/friend-packages.txt;
/shared/data/ccvs/repository/ide/golden/friend-packages.txt,v  <--  
friend-packages.txt
new revision: 1.31; previous revision: 1.30
done
Checking in ide/golden/cluster-deps.txt;
/shared/data/ccvs/repository/ide/golden/cluster-deps.txt,v  <--  
cluster-deps.txt
new revision: 1.51; previous revision: 1.50
done
Checking in core/bootstrap/manifest.mf;
/shared/data/ccvs/repository/core/bootstrap/manifest.mf,v  <--  manifest.mf
new revision: 1.8; previous revision: 1.7
done
Checking in core/bootstrap/nbproject/project.xml;
/shared/data/ccvs/repository/core/bootstrap/nbproject/project.xml,v  <--  
project.xml
new revision: 1.13; previous revision: 1.12
done
Checking in nbbuild/build.properties;
/shared/data/ccvs/repository/nbbuild/build.properties,v  <--  build.properties
new revision: 1.404; previous revision: 1.403
done
Checking in nbbuild/cluster.properties;
/shared/data/ccvs/repository/nbbuild/cluster.properties,v  <--  
cluster.properties
new revision: 1.142; previous revision: 1.141
done
Checking in utilities/nbproject/project.xml;
/shared/data/ccvs/repository/utilities/nbproject/project.xml,v  <--  
project.xml
new revision: 1.20; previous revision: 1.19
done
Checking in utilities/nbproject/project.properties;
/shared/data/ccvs/repository/utilities/nbproject/project.properties,v  <--  
project.properties
new revision: 1.18; previous revision: 1.17
done
RCS 
file: /shared/data/ccvs/repository/utilities/src/META-INF/services/org.netbeans.spi.sendopts.OptionProcessor,v
done
Checking in 
utilities/src/META-INF/services/org.netbeans.spi.sendopts.OptionProcessor;
/shared/data/ccvs/repository/utilities/src/META-INF/services/org.netbeans.spi.sendopts.OptionProcessor,v  
<--  org.netbeans.spi.sendopts.OptionProcessor
initial revision: 1.1
done
Removing utilities/src/META-INF/services/org.netbeans.CLIHandler;
/shared/data/ccvs/repository/utilities/src/META-INF/services/org.netbeans.CLIHandler,v  
<--  org.netbeans.CLIHandler
new revision: delete; previous revision: 1.1
done
Checking in utilities/src/org/netbeans/modules/openfile/Handler.java;
/shared/data/ccvs/repository/utilities/src/org/netbeans/modules/openfile/Handler.java,v  
<--  Handler.java
new revision: 1.3; previous revision: 1.2
done
Checking in utilities/src/org/netbeans/modules/openfile/Bundle.properties;
/shared/data/ccvs/repository/utilities/src/org/netbeans/modules/openfile/Bundle.properties,v  
<--  Bundle.properties
new revision: 1.39; previous revision: 1.38
done
IDE: [18.9.06 19:15] Committing finished