Skip to content
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

Merged
merged 6 commits into from
May 20, 2017
Merged

Add prompt and dat.json to dat create #765

merged 6 commits into from
May 20, 2017

Conversation

joehand
Copy link
Collaborator

@joehand joehand commented May 17, 2017

With this PR dat create will now:

  • prompt users to add title, description to dat.json (reads any dat.json already present)
  • Asks user if they want to import.

With Importing example:

screen shot 2017-05-16 at 19 25 07

Not importing example:

screen shot 2017-05-16 at 19 23 24

@joehand joehand added this to the SLEEP - Public Release milestone May 17, 2017
}
}
prompt.message = chalk.green('> ')
prompt.delimiter = '' // chalk.cyan('')
Copy link
Collaborator

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?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Unfortunately it adds a delimiter after the message and after the description:

screen shot 2017-05-17 at 10 33 56

(In this case, I have My dataset in dat.json already, so it defaults to that).

I could change the message to something that'd make more sense with a delimiter.

prompt.get(schema, writeDatJson)

function writeDatJson (err, results) {
if (err) return console.log(err.message) // prompt error
Copy link
Collaborator

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

Copy link
Collaborator

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?

Copy link
Collaborator Author

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))}
Copy link
Collaborator

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.

Copy link
Collaborator Author

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) {
Copy link
Collaborator

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?

This was referenced May 17, 2017
@joehand joehand merged commit 1343821 into master May 20, 2017
@joehand joehand deleted the dat-create branch May 20, 2017 00:02
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.

2 participants