Skip to content

add dartfmt support #202

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

Merged
merged 3 commits into from
Apr 23, 2015
Merged

add dartfmt support #202

merged 3 commits into from
Apr 23, 2015

Conversation

devoncarew
Copy link
Contributor

Fix #168.

@seaneagan

Write a wrapper around the dartfmt SDK tool. This is scoped to just exposing what's in the SDK .The user still has the option of using a PubApp wrapper around the dart_style tool.

(the Analyzer and Dart2js classes are unchanged; they just changed locations within the file)

@@ -33,6 +33,51 @@ Directory getSdkDir([List<String> cliArgs]) => cli_util.getSdkDir(cliArgs);

File get dartVM => joinFile(sdkDir, ['bin', _sdkBin('dart')]);

/// Utility tasks for for getting information about the Dart SDK and invoking
/// dart applications.
class Dart {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this class be called Dart or DartSdk?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DartVM?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I chose Dart since it wraps the dart executable, similar to how Pub wraps pub and Dart2js wraps dart2js. However we do have Analyzer for dartanalyzer, so we could do Dart2js -> Compiler and Dart -> Vm, but I kind of think the Dart prefix is nice.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, it might be interesting to namespace all the sdk stuff, so:

class Sdk {

  /// Defaults the path to the current SDK.
  Sdk([String path]);

  static Directory get dir;

  final Vm vm;
  final Compiler compiler;
  final Analyzer analyzer;
  final Formatter formatter;
}

final Sdk sdk = new Sdk();
print('Sdk dir: ${sdk.dir}');
sdk.vm.run(...);
sdk.compiler.compile(...);
sdk.analyzer.analyze(...);
sdk.formatter.format(...);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left this as Dart, and removed the location static field. That would apply to a wrapper around the dartsdk, but not one around the dartvm.

@seaneagan
Copy link
Contributor

Filed my review comments as #204, let's discuss there.

devoncarew added a commit that referenced this pull request Apr 23, 2015
@devoncarew devoncarew merged commit 60a9e68 into master Apr 23, 2015
@devoncarew devoncarew deleted the devoncarew_dartfmt branch April 23, 2015 21:18
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

Successfully merging this pull request may close these issues.

Request: default task: format
2 participants