Store user answers as defaults

[kwakayama]

We added the ability to save user inputs as new default values.

To use this feature just set the store attribute to true.

var prompts = [{
name: 'authorName',
message: 'Author\'s Name',
store: true
}];

When the user enters his name (eg.: octocat), the generator will return the name as default answer next time.

All prompt types of Inquirer are supported.

We use Yeoman's storage api to save the values.
The values will be stored in a settings.json which is located in the root of the generator.

You can see a working example in the generator-node-gulp.

[blai]

+1 but didn't we have .yo-rc.json already? Might as well use that for this purpose.

[kojiwakayama]

+1

[stefanbuck]

@blai The .yo-rc.json isn't the right place for our purpose, because we want something like a generator settings and the .yo-rc.json is just for project specify settings. Another reason is the location of these files. The .yo-rc.json is located in the root of a project and this empty during initial setup process. Therefore we've decided to introduce a new setting storage in the root of the generator.

[timoweiss]

👍

[eddiemonge]

We've talked about this before in the team. We would like to have a 'global' yo-rc.json settings file that stores data and then each generator can override this global with its on yo-rc.json file. Im not sure this satisfies that requirement but is along the same idea I think.

I agree that settings.json isn't the best name for the file. Where does it get stored? It looks like in the project directly.

[stefanbuck]

The intention of this PR is to configure the generator itself. The PR comes along with two features:

1. A settings store for each generator to store generator specific stuff
2. A new prompt property store to save the entered values in the settings store

Settings store

The settings store behaves like the config store but the store file is located in the root of the used generator eg. /usr/local/lib/generator-node-gulp. This is needed to have the settings for one generator in one place.

Prompt property store

Like in the example below from @kwakayama you can see how to setup the prompt with the new store property. The enterd prompt values will be stored in the settings store of the generator. When the user runs the generator again to set up the next project, the prompt will be prefilled with the last entered values.

[SBoudrias]

Why is this test here?

[stefanbuck]

Because this method was untested and i couldn't find any tests for it

[SBoudrias]

Then it should be in its own describe :)

[SBoudrias]

Ok so, I'm pretty happy with all of this.

My only concerns is that settings and store is not very descriptive. Maybe the names should relate more to words like global and cache.

[SBoudrias]

It'd be better (more explicit) if this function returned the Storage instance and if the settings attribute was assigned inside the constructor method. (so a rename like _getSettingsStorage())

this.settings = this._getSettingsStorage();

I know that is not how the other is done, my bad. So if you can also fix the _setStorage method, that'd be great.

[stefanbuck]

We will update both calls

[eddiemonge]

I dont think Im happy with creating a file in a generator's folder. By /usr/local/lib/generator-node-gulp do you mean in the actual npm global installed location for the generator? Something like /usr/local/lib/node_modules/generator-node-gulp? If so, I don't think we should be putting non-npm created files there. They should still be written to a global .yo-rc.json file in the users home directory to be safer.

[SBoudrias]

It is true there's an issue with writing in the npm dir where we might not have write permissions without sudo.

[SBoudrias]

There's another issue we need to resolve before this feature can hit production. We need a built-in way for user to opt-out of the cache. I guess the best way will be to implement a standardized option like --skip-install.

[blai]

+1 to opt-out, maybe --reset-settings?

[SBoudrias]

Oh, --no-cache or --skip-cache would do. By doing so, the cache values could just be overwritten - meaning there's no need for an actual reset option.

[stefanbuck]

@eddiemonge yes, currently the settings file locations is the global installed location of the generator.
To store the file outside is definitely a good idea, and after a while, i like the idea with the users home dir. Only the name .yo-rc.json isn't also very descriptive for me. Some suggestions from my side:

1. .yo-generator-settings.json
2. .yo-gen-settings.json
3. .yo-store.json
4. .yo-preferences.json
5. .yo-pref.json

[SBoudrias]

In fact, even if we wanted to, we cannot reuse .yo-rc.json because this file mark the root of a project.

I'd probably go with ~/.yeoman-cache.json

[eddiemonge]

Why not just ~/.yeomanrc ?

[sindresorhus]

I would go with .yo-rc-global.json.

[stefanbuck]

.yo-rc-global.json sounds good to me. Any objections? Otherwise I will rename settings.json to it.

[kwakayama]

+1 for .yo-rc-global.json

[kojiwakayama]

+1 for .yo-rc-global.json

[stefanbuck]

Tomorrow i will implement the opt-out argument --skip-cache to complete the PR and get it back to you 😉

[SBoudrias]

Allright! Thanks for the awesome works.

By the way, we need to release a couple of fixes and let the current version run a bit before we launch 0.18 with this new feature. Don't worry if it takes a week or two before merging in master.

[stefanbuck]

We are done from our side. It's your turn now 😉

[blai]

🍻 Nice work

[eddiemonge]

@stefanbuck could you squash the commits down to one?

[kojiwakayama]

thanks for the awesome work. very useful.

[kojiwakayama]

@eddiemonge , what's wrong with the commits ?

[SBoudrias]

@kojiwakayama The issue is there's 19 commits and most of them are not relevant on their owns: http://yeoman.io/contributing/pull-request.html

A little clean up sure wouldn't hurt.

[kojiwakayama]

@SBoudrias, agreed, a little clean up makes sense. Squashing the history down to one doesn't.

[eddiemonge]

Well ideally the PR would only have one feature in it with one commit for that feature. That doesn't seem to be the case here so a few commits should be ok but one would be better so we could track the changes if needed. One commit is easier when using git bisect than 19.

[stefanbuck]

@eddiemonge @SBoudrias I've squashed the commits down to one as your prefer.

[sindresorhus]

@stefanbuck Can you fix the merge conflict?

@SBoudrias @eddiemonge Anything left for this to be merged?

[eddiemonge]

typo

[eddiemonge]

Only other thing I would like to see is some documentation added about whats happening over at https://github.com/yeoman/yeoman.io/

[SBoudrias]

@sindresorhus we're going to release a patch for 0.17, wait a bit then release this one on version 0.18

[eddiemonge]

I think this is ready now.

[SBoudrias]

@eddiemonge Not sure... I'd like to land #617 - make a patch release and finish generator-generator on 0.17 before we start looking at 0.18

[SBoudrias]

Update! Patch release for yeoman-generator went out and the new generator-generator went out too!

This mean we should consider merging this early next week! And release 0.18 soon after. BTW, I'll take care of the merge because I want to review before that this patch doesn't break the existing Inquirer public API.

[SBoudrias]

Hey guys, sorry if it took that long. I realized reviewing this review again last week that it actually break the Inquirer public API as it don't allow function as values or choices.

That got me thinking and I believe we need to reworks this PR. The main point is that we really need to make the feature less intrusive. This could be done in two step:

1. Before calling inquirer.prompt, check if some prompts answer are cached. If so (and the cache should use), remove those objects form the array of questions we're going to feed inquirer.
2. After inquirer ran (either in the callback or on each question), retrieve cached values and add them to the answer object. Then, cache new answers if necessary.

Doing it this way, we're making sure we're not overwriting Inquirer behavior and we're just applying a thin layer on top of it.

Another concern I have, how can we clear the global cache? What if a generator change a question? ATM I feel an answer is cached forever, should we bind the cache to the specific generator version? (Open questions)

[stefanbuck]

Mhh okay, so i will start reworking the PR in the next days. In regard to your convern with clearing the cache: this is a good point, we missed in the current version. You are right, at the moment the anwers will cached forever. Maybe yeoman cli should provide a command to clear the cache manually for a specifc generator. To resolve the problem with changing questions I’ve following approaches:

• Bind answer to a specify generator version
• Clear answers on generator update
• Migration Script (maybe hard to maintain and explain)

Personally i prefer the frist or the second approach because both are pretty simple and the generator developer doesn’t need to know anything about the answer cache.

[stefanbuck]

@sindresorhus why did you merged this PR? It's still in progress! Yesterday I've rejected all our changes to have a clean repo for the reimplementation.

[sindresorhus]

No idea what's happening, but I didn't. See the referenced commit is something completely different: 7a4ce2e

[SBoudrias]

Wow, weird Github bug...

[SBoudrias]

Hey @stefanbuck got an answer from Github staff, this is effectively a bug (the last commit pushed may have been corrupted during transfer or something like that and it broke Github). Can you just reopen a new PR?

[stefanbuck]

Yes I do but need time to do this.