Skip to content

Refactor grinderArgs() #116

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

Open
seaneagan opened this issue Mar 12, 2015 · 10 comments
Open

Refactor grinderArgs() #116

seaneagan opened this issue Mar 12, 2015 · 10 comments
Assignees
Milestone

Comments

@seaneagan
Copy link
Contributor

The purpose of grinderArgs() it is to be able to access custom command-line arguments from tasks. However, package:args currently doesn't allow unknown options to be passed anyway (arg parsing will fail). Ideally a task could declare its own custom command-line options and flags. That would allow:

  • including help text for them via grind -h
  • validating custom command-line arguments with task arguments.

It would be really cool to reuse unscripted's Flag and Option annotations:

@Task('Deploy the project.')
deploy(
  @Option(allowed: const ['local', 'ci', 'test', 'prod'], help: 'Which environment to deploy to.')
  String env) {
  // ...
}

Then grind -h would include something like:

[deploy] Deploy the project.
  --env    Which environment to deploy to.

If we do keep grinderArgs(), I think it should be a getter, and probably move it to GrinderContext, or integrate with whatever solution we do for dart-lang/core#31.

@devoncarew
Copy link
Contributor

grinderArgs is mostly used to be able to locate the dart sdk from tasks. If we're not able to discover it via the current path (or which), then we fall back on looking for a --dart-sdk cli option.

Refactoring it's meaning sounds good. We may want to hang the cli args on an instance of a Grinder object though, instead of using a global var. That was an expedient solution -

@seaneagan
Copy link
Contributor Author

Also, I think it'd be great to support positional arguments like this:

@Task('Deploy the project.')
deploy(
  [@Positional(allowed: const ['local', 'ci', 'test', 'prod'], help: 'Which environment to deploy to.')
  String env = 'local']) {
  // ...
}

Which can be used in the CLI as:

grind deploy (defaults to 'local')
grind deploy:prod

@devoncarew devoncarew modified the milestones: 0.7.1, 0.7.2 May 18, 2015
@seaneagan seaneagan self-assigned this May 27, 2015
@seaneagan
Copy link
Contributor Author

I'll take this. I'm going to add support for positional args first e.g. grind bump:patch. Those are much more straight-forward since they are syntactically attached to individual tasks. For named args we'll need to decide whether to support something similar e.g. grind bump:type=patch:pre=true or traditional named args e.g. grind bump --type=patch --pre

@zoechi
Copy link

zoechi commented May 27, 2015

I'm in favor of the traditional style.

@seaneagan
Copy link
Contributor Author

The disadvantage with the traditional style is that they are not syntactically scoped to individual tasks, yet they are meant to be consumed by individual tasks. So for one thing there can be conflicts between different tasks. However, traditional named args allow passing args to dependency tasks which don't show up on the command line.

@devoncarew
Copy link
Contributor

I'll take this.

Awesome! This will be great to have.

@zoechi
Copy link

zoechi commented May 28, 2015

+1
@seaneagan You know this topic better than me. For me it was just - another "syntax" to learn.

@seaneagan
Copy link
Contributor Author

I'm going to go with normal --options=... and --flags. However, I'm going to need to fix dart-lang/core#107 first to make it work.

@seaneagan
Copy link
Contributor Author

Update: I'll be starting on this as soon as I finish seaneagan/unscripted#40 which will allow more dynamic sets of options in unscripted, which is needed here.

@seaneagan
Copy link
Contributor Author

Starting on this. Rough plan:

  • export Option, Flag, Positional from unscripted
  • Add fields to GrinderTask to define their Options and Positionals (and a way to set/initialize them).
  • Add logic to discover tasks with Option and Positional annotations on parameters (ala unscripted).
  • Combine all options into a (hidden) option Group.
  • Update unscripted positional params to take positional (and named?) values e.g. bump:patch,pre=true
  • Add TaskInvocation class to defines an invocation of a named task with named and positional arguments. Define == and hashCode based on these fields.
  • Change GrinderTask.execute to take a TaskInvocation. Possibly move execute to Grinder.executeTask.
  • Allow tasks to depend on TaskInvocations instead of just tasks both in imperative and annotation APIs.
  • Parse out a list of TaskInvocations instead of a list of GrinderTasks from command-line args.
  • Pass those to Grinder.start which should add dependency `TaskInvocations.
    • Disallow different invocations of the same task. (I think)
  • Combine positional and named arguments received via unscripted into list of TaskInvocations to execute.
  • Add help output for Options and Positionals for each task.
  • Add tab-completion for Options and Positionals for each task.

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

3 participants