Request: default task: format

[sethladd]

I'd love it if grinder had a default talk for 'format' which ran dart_style's dartfmt over my project.

Thanks!

[zoechi]

It's quite easy to use pub global run from grinder (does activate automatically)
I created dart-lang/dart_style/#215 to be able to use the API but currently I think pub global run is the better way anyway because it doesn't need an additional dependency in pubspec.yaml

[devoncarew]

Yeah, currently using it would look something like this:

PubApp dartStyle = new PubApp.global('dart_style');
dartStyle.run(['foo', 'bar'], script: 'dartfmt'); // or whatever args make sense to dartfmt

It will take care of activating the tool if it does not exist on the system.

I'd be happy w/ a PR that added a semantic wrapper around the dart_style cli tool. Something like what we have now for the analyzer and pub. This would lower the bar for people to use it in their build scripts, but definitely not a requirement for them to use it.

[devoncarew]

Although, if dartfmt is generally run from the sdk, and not via pub global activate, then we should have a 1st class wrapper around it in grinder (and one that doesn't drive it via pub global activate).

[seaneagan]

IMO, the fact that dartfmt still exists in the SDK is merely a backwards compatibility thing. It should be much preferred to run it via pub global activate in order to control the version at a project level, rather than a system level, and in particular to always be able to get the latest version. Having a Dartfmt.format driven by pub global activate could be ok, but it would kind of give the impression that it is being run via the SDK since it's similar to the other SDK executable wrappers.

[sethladd]

Regardless, I think grinder should ship with a dartfmt task. Since dartfmt ships with the SDK, we can assume it exists. Could the built-in dartfmt task just use whatever is on the command line?

[seaneagan]

It would be a bit more discoverable if dart_style and format were consistent:

new PubApp.global('formatter').run([arg])

[seaneagan]

@sethladd

Are you more interested in validating the formatting or actually formatting?

By default would it run over all dart files in the project?

Would it require glob support for non-default use cases? (dart_style doesn't appear to support this natively yet)

Do you want to be able to pass command-line arguments (#116 and #175) for which files to format?

[seaneagan]

If we depend on what's on the command-line, that's unpredictable. I think the user should be explicit about which version they want. Could be two separate constructors:

new Dartfmt.sdk().format(...)
new Dartfmt.pub().format(...)

Or if dartfmt supported --version, then the user could specify a version constraint, and it could first check if the pub version is already activated, and if that matches the version constraint, then use it, otherwise check if the SDK dartfmt matches the version constraint, otherwise activate the pub version with that version constraint (see #165):

new Dartfmt(version: '^0.1.7').format(...)

[zoechi]

Maybe DartFormat could be just a subclass of PubApp where the name ('dart_style') is predefined, to ensure the API stays consistent

[seaneagan]

@zoechi That might work, although the Dartfmt.run method would not be nearly as useful as e.g. a Dartfmt.format(dryRun: true) or similar method, so we'd at least want to include that as well. I guess Dartfmt.isActivated/activate/runcould use the logic described above to determine whether to use sdk or pub versions.

If we go with constructor(s) i.e. new Dartfmt ..., then we may want to update the other interfaces to e.g. new Dart2js ..., new Analyzer ..., etc.

[sethladd]

@seaneagan two tasks: validate formatting, and actually do the formatting.

I'd like to run: grind format or grind format:validate or whatever the command-line format is.

Ultimately, I'm trying to create tasks and workflows to automate my day-to-day source code and development and test and release workflows.

[devoncarew]

Lets plan on having the default formatting functionality in grinder be a wrapper around the sdk tool - bin/dartfmt. This will mirror what we do with dart2js, pub, the dart VM, and the analyzer.

If we want to have a formatter with a similar API, backed by an activated dart_style package, we could do that. But I think that would have to be something that the user would explicitly request. The sdk one should be the default for consistency with the other sdk tools.

[seaneagan]

Now that we have the ability to ship dartfmt outside the SDK, users don't have to wait for SDK updates to update it, which is great. And looks like dartanalyzer is heading that way too! So why not make it at least as easy if not easier to use the latest versions from pub than the older SDK versions? What do you think about the idea of giving the user options:

class Dartfmt extends PubApp {
  /// By default, uses the latest version from pub.
  /// 
  /// If [version] is specified, then if a matching version from pub has already been activated, 
  /// use it, else if the version in the SDK matches, use it, else activate a matching version from pub.
  Dartfmt({VersionConstraint version})

  /// Uses the SDK version.
  Dartfmt.sdk();
}

It might even make sense at some point to remove those executables from the SDK proper, and the various OS dart packages (homebrew, chocolatey, docker, etc.) could still install them by running the following if desired:

pub global activate dart_style <version>
pub global activate analyzer_cli <version>

[devoncarew]

It's exposed in the source in the repo; we may change how it's exposed as per #204.