Skip to content
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

Define a public API for CommandLineArgumentParser and CommandLineProgram #127

Open
cmnbroad opened this issue Jan 22, 2018 · 2 comments
Open
Assignees

Comments

@cmnbroad
Copy link
Collaborator

In order to allow consumers of CommandLineArgumentParser to have greater control over parser behavior, we should make the following changes:

  • Define a public interface for CommandLineArgumentParser and allow it to be subclassed, possibly including a factory for ArgumentDefinition (see CommandLineArgumentParser ArgumentDefinition class should be factored out #9).
  • Define the CommandLineProgram base class (see Add CommandLineProgram interface #96), and include an overridable implementation of getCommandLineParser(). This would allow tools to provide specialized behavior for the parser.
  • Refactor/replace SpecialArgumentsCollection to allow the consumer to control the names of the arguments that are visible to users, and address some current issues (like Barclay needing to know how to display a toolkit’s version).

This would provide a generalized mechanism for allowing customization, and help enable solutions for a number of issues such as (i.e. #126, #123, and possibly #34 and #106) by allowing the consumer to provide specialized behavior through method overrides. It would require a major version bump, although it may be possible to do in a minor version bump by providing new classes and leaving the old ones in place as deprecated.

@cmnbroad
Copy link
Collaborator Author

The CommandLineProgram base class should include methods that handle some of the warning messages as implemented in broadinstitute/gatk#4429. Barclay should also include the TerminalColor some variant of class mentioned in broadinstitute/gatk#4429 (comment).

@cmnbroad
Copy link
Collaborator Author

@magicDGS To update the PR for this, I'd start by looking at all the comments above, and at any code that is or should be common between the Picard and GATK implementations (beta/experimental decorations, tool registry). I also think things like getToolkitName and getToolkitVersion should be included. And we still need to resolve the Object/int return type issue. This could take some time to get consensus on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant