-
-
Notifications
You must be signed in to change notification settings - Fork 450
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
Add prompt and dat.json to dat create #765
Conversation
src/commands/create.js
Outdated
} | ||
} | ||
prompt.message = chalk.green('> ') | ||
prompt.delimiter = '' // chalk.cyan('') |
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.
maybe :
would be better to read?
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.
src/commands/create.js
Outdated
prompt.get(schema, writeDatJson) | ||
|
||
function writeDatJson (err, results) { | ||
if (err) return console.log(err.message) // prompt error |
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.
this should exit with code 1 too
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.
would the bus.emit('exit:error', err)
do that?
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.
Yes! At first I thought this may be prompt errors, e.g. if you get something that doesn't match the required pattern, but I think that is handled differently.
would the bus.emit('exit:error', err) do that?
yep
src/ui/create.js
Outdated
var progressView | ||
var exitMsg = ` | ||
Your dat is created! Run ${chalk.green('dat sync')} to share: | ||
${chalk.blue('dat://' + stringKey(dat.key))} |
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.
template literals all the way?
${chalk.blue(`dat://${stringKey(dat.key)}`)}
I know it can look confusing to nest them, but it's also weird to mix two concepts that can do the same.
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.
Ha ya, I'm gonna make the key a ui/element
along with pluralize. Just got a bit lazy last night.
src/ui/create.js
Outdated
${state.exiting ? exitMsg : chalk.dim('Ctrl+C to Exit')} | ||
` | ||
|
||
function pluralize (str, val) { |
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.
this function has been defined twice in this patch, maybe we want to move it to lib/
or source a simple one from npm?
With this PR
dat create
will now:With Importing example:
Not importing example: