-
Notifications
You must be signed in to change notification settings - Fork 6
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
Implement #2 #13
Implement #2 #13
Changes from 2 commits
56ed262
54b8267
213df97
1764afc
62f772b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,92 @@ | ||
|
||
library den.src.commands.spec; | ||
|
||
import 'dart:async'; | ||
import 'dart:mirrors'; | ||
import 'dart:io'; | ||
|
||
import 'package:path/path.dart' as p; | ||
import 'package:unscripted/unscripted.dart'; | ||
import 'package:prompt/prompt.dart'; | ||
import 'package:pub_semver/pub_semver.dart'; | ||
import 'package:yaml/yaml.dart'; | ||
|
||
import '../pub.dart'; | ||
import '../theme.dart'; | ||
|
||
class SpecCommand { | ||
|
||
Pubspec _pubspec; | ||
InstanceMirror get _pubspecReflection => reflect(_pubspec); | ||
YamlMap get _yamlMap => _pubspec.yamlMap; | ||
final fields = ['name','version','author','homepage','description']; | ||
|
||
List<Question> get questions => | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this still used? |
||
fields.map((String field){ | ||
return new Question("$field (${_yamlMap[field]})"); | ||
}); | ||
|
||
@SubCommand(help: 'Initiate or edit pubspec.yaml file.') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "pubspec.yaml file" -> "a pubspec" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "Initiate" -> "Initialize" |
||
spec( | ||
|
||
) { | ||
_prompts(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: not sure why _prompts() is a separate method. |
||
} | ||
|
||
Future _prompts() { | ||
var pubspecFile = new File('pubspec.yaml'); | ||
_pubspec = pubspecFile.existsSync() ? Pubspec.load() : Pubspec.init(); | ||
|
||
return Future.forEach( | ||
fields, | ||
(String field){ | ||
var defaultValue = _yamlMap[field] != null ? _yamlMap[field] : ''; | ||
|
||
var question = defaultValue != '' | ||
? new Question("${theme.question(field)} (${theme.questionDefault(defaultValue)}):", defaultsTo: defaultValue) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove the ":", it should already be included in the prompt output. |
||
: new Question(theme.question(field), defaultsTo: ''); | ||
|
||
return ask(question) | ||
.then((String response){ | ||
if(response=='') _pubspecReflection.setField(new Symbol(field), defaultValue); | ||
else _pubspecReflection.setField(new Symbol(field), response); | ||
}); | ||
} | ||
).whenComplete((){ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. whenComplete -> then (otherwise errors are swallowed) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same for other uses of whenComplete |
||
|
||
var contents = _syntaxHighlighted(); | ||
print("\npubspec.yaml\n============\n$contents\n"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. print('''
pubspec.yaml
===========
$contents
'''); |
||
return ask(new Question.confirm("All correct?", defaultsTo:true)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove the "?" (there's already one in the left column) |
||
.then((bool correct){ | ||
if(!correct) return _prompts(); | ||
else { | ||
_pubspec.save(); | ||
return close(); | ||
} | ||
}); | ||
}); | ||
} | ||
|
||
String _syntaxHighlighted() { | ||
var fieldContents = fields.toSet().intersection(_yamlMap.keys.toSet()).map( | ||
(String field) { | ||
return "${theme.field(field)}: ${theme.value(_yamlMap[field])}"; | ||
} | ||
).join("\n"); | ||
|
||
var envContents = _yamlMap['environment'] != null ? _block('environment') : ''; | ||
var depContents = _yamlMap['dependencies'] != null ? _block('dependencies') : ''; | ||
var devdepContents = _yamlMap['dev_dependencies'] != null ? _block('dev_dependencies') : '';; | ||
|
||
return "$fieldContents$envContents$depContents$devdepContents"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we can assume this order or that there are not other fields to include. I think it needs to go through the contents of the _yamlMap and generically highlight things. I'd be ok with saving the semantic highlighting for later though. |
||
} | ||
|
||
String _block(String title) { | ||
var lines = _yamlMap[title].keys | ||
.map((String key) { | ||
var value = "'${_yamlMap[title][key]}'"; | ||
return "${theme.dependency(key)}: ${theme.value(value)}"; | ||
}).toList(); | ||
return "\n${theme.field(title + ':')}\n${lines.map((line) => ' $line').join('\n')}"; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,6 +11,8 @@ import 'package:pub_semver/pub_semver.dart'; | |
import 'package:yaml/yaml.dart'; | ||
|
||
import 'yaml_edit.dart'; | ||
import 'git.dart'; | ||
|
||
|
||
class Pubspec { | ||
|
||
|
@@ -25,14 +27,33 @@ class Pubspec { | |
String get path => _path; | ||
final String _path; | ||
String get name => _yamlMap['name']; | ||
set name(String _name) { | ||
contents = setMapKey(_contents, _yamlMap, 'name', _name, false); | ||
} | ||
String get author => _yamlMap['author']; | ||
set author(String _author) { | ||
_setValue('author', _author); | ||
} | ||
Version get version => new Version.parse(_yamlMap['version']); | ||
set version(Version v) { | ||
contents = setMapKey(_contents, _yamlMap, 'version', v.toString(), false); | ||
} | ||
String get homepage => _yamlMap['homepage']; | ||
set homepage(String _homepage) { | ||
_setValue('homepage', _homepage); | ||
} | ||
String get documentation => _yamlMap['documentation']; | ||
String get description => _yamlMap['description']; | ||
set description(String _description) { | ||
_setValue('description', _description); | ||
} | ||
void _setValue(String key, String value) { | ||
if(value!=null && value!='') { | ||
contents = setMapKey(_contents, _yamlMap, key, value, false); | ||
} else { | ||
contents = deleteMapKey(_contents, _yamlMap, key); | ||
} | ||
} | ||
VersionConstraint get sdkConstraint { | ||
var env = _yamlMap['environment']; | ||
if (env == null) return VersionConstraint.any; | ||
|
@@ -87,6 +108,19 @@ class Pubspec { | |
this._contents, | ||
this._yamlMap); | ||
|
||
static Pubspec init() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make this return a |
||
var packageRoot = p.current; | ||
var pubspecPath = p.join(packageRoot, _PUBSPEC); | ||
var contents = "name: ${p.basename(packageRoot)}"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be nice to strip off any ".dart" or "-dart" suffix here. |
||
var yaml = loadYamlNode(contents, sourceUrl: pubspecPath); | ||
var _pubspec = new Pubspec(pubspecPath, contents, yaml); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove the "_", no need to make a local variable private. |
||
|
||
_pubspec.version = new Version.parse('0.1.0'); | ||
|
||
if(checkHasGitSync()) _pubspec.author = "${gitConfigUserNameSync()} <${gitConfigUserEmailSync()}>"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Need some error handling here. If git fails, or if the git config for either field was not set, it should not set the author. |
||
|
||
return _pubspec; | ||
} | ||
static Pubspec load([String path]) { | ||
var packageRoot = _getPackageRoot(path == null ? p.current : path); | ||
var pubspecPath = p.join(packageRoot, _PUBSPEC); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just noticed dart editor has author before version, let's switch to that to match.