Feat Path Options

[marani]

Changelog

(diff looks huge but most of them are tests)

Generator Changes

• Add options.json to define options with Yeoman options. 4 new options:

  --app-path
  --dist-path
  --e2e-path
  --tmp-path

• Refactor init -> constructor to make options definition available to yo gulp-angular --help
• Move appName formatting to format.js instead of init.
• Add retrieveOptions in index.js to parse options in cases with no --default & no skipConfig. This function for options is similar to askQuestions for prompts.
• Add mock-options.js with a defaults value to make it similar to prompts, for easy use in tests.
• Add utils.js for some path processing functions.
• Add a property to props: props.paths is a hash, where keys are folder in generator template & values are dest folder for generated app.

  {
    `src`: `public`,
    `e2e`: `tests/e2e`,
    `tmp`: `.tmp`,
    `dist`: `dist`
  }

• Use utils.replacePrefix() for each dest str in write.js

Tempate changes

• All static paths are replaced with template variables or gulp variables.
• Gulp refers to .yo-rc.json to get paths data.

Test Changes

• Change some structures in test-gulp-task-inception.mocha.js & test-files-generate.mocha.js to enable different options for each case.
• Fix test suite message, differentiate between options & prompts. Options are flags: --skip-install, --app-path=..., prompts are questions.
• Add 1 case for test-gulp-task-inception.mocha.js with different paths.
• Add 1 case for test-files-generate.mocha.js with different paths.
• Add tests-utils.js.

Mics

• Update README.md.
• Commits are already squashed & rebased to master, all protractor tests also passed.

Further refactor suggestion:

• There is now options for each case so test-options.mocha.js should just be merged into test-files-generate.mocha.js
• test-write-files is quite troublesome, there are cases that file is both dotfile & template file, or a double dash __ file is a template file that will break the assertion. Plus the assertion isn't really important as most file assertions are already done in test-files-generate.mocha.js. So I suggest remove this test.

[zckrs]

👍 to remove test-write-files.js

[marani]

Should I do it now or another PR? It is actually blocking me from implementing template on gitignore. Also I would like to remove dotfiles in files.json, merge them into either staticFiles or templates, then rename the file to _.gitignore. Something like this:

{
  "staticFiles": [
    ".bowerrc",
    ".editorconfig",
    ".jshintrc",

    "gulpfile.js",
    "gulp/e2e-tests.js",
    "gulp/proxy.js",
    "src/favicon.ico",
    "src/404.html",
    "src/assets/images/yeoman.png",
    "karma.conf.js",
    "protractor.conf.js",
    "e2e/main.spec.js",
  ],
  "templates": [
    ".gitignore",

    "package.json",
    "bower.json",
    "e2e/main.po.js",
    "gulp/build.js",
    "gulp/consolidate.js",
    "gulp/watch.js",
    "gulp/wiredep.js",
    "gulp/server.js",
    "gulp/unit-tests.js",
    "src/index.html",
    "src/app/main/main.controller.spec.js"
  ]
}

[Swiip]

I like how this feature evolve. Organizing and documenting options seams necessary now.

I still have a remark about the way the paths are stored. Referring to the .yo-rc is strange to me. As asked in the #203, I better imagine this configuration in the gulp files, why not directly in the root gulpfile.js. A new user will better understand his gulp process like this I think.

Another remark. In the options.json you can't have the --advanced which is not merged yet but I don't see either the --default which is already there.

[marani]

About the --default you are right I forgot that, fixing now.

For the problem of where to store paths. There are actually 2 modules that need the paths setting. The first are gulpfiles. The second are future sub-generators. So the reasoning is:

For .yo-rc.json

• Good: Reusable by sub generators. The situation will be clearer once we can write sub generators from project directory in the future when this yeoman feature comes out. Also it makes paths setting persists if we re-use .yo-rc.json.
• Bad: gulp files needs to read from there, which is quite strange as you said & that's true.

For storing in gulpfile.js

• Good: gulp files depend on gulpfiles.js only -> more predictable & it feels easier to change.
• Bad: Sub generators have to read paths data from gulpfile.js, which is equally strange.

Storing in both place makes duplication, and it's the worst so I wouldn't mention here.

.yo-rc.json at first isn't an intuitive place to edit things, but people are actually doing that in generator-backbone.
Also compare in terms of scope, yo-rc.json should have a higher scope than gulp files. So that's why I chose yo-rc.json instead of some gulp file.

Another solution I have thought about is to make gulpfile.js read from yo-rc.json then all sub tasks gulp file read from gulpfile.js. If you agree I will refactor into something like that.

[zckrs]

I prefer stores un yo-rc.json. As you said yo-rc.json have a higher scope in yeoman generator

[zckrs]

Can you remove test-write-files.js and rebase ?

Ans you can delete dotfileFiles.

[marani]

Sure I'm working on it.

[marani]

• Added default to options definition.
• Remove test-write-files & take away dotfiles from files.json.
• Protractor tests passed.

One last thing is the exception of vendor style in write.js, it's not really nice. I'm thinking of a way to make it look better, any suggestion?

[marani]

Ok now it's nice.

Now I am refactoring the test one more time for the better. I am making optionCase & promptCase cleaner & more readable by putting gulpAngular.mockPrompt into beforeEach rather than on top of it blocks like what it is now, then move promptCase definition into before blocks.

    helpers.testDirectory(path.join(__dirname, folderName), function (err) {
      if (err) {
        done(err);
      }

      gulpAngular = helpers.createGenerator(
        'gulp-angular:app',
        [
          '../../app',
        ],
        false,
        optionCase
      );
      gulpAngular.mockPrompt(promptCase);

      gulpAngular.on('run', outputInTest.mute);
      gulpAngular.on('end', outputInTest.unmute);

      done();
    });

    before(function() { 
      optionCase = _.assign(JSON.parse(JSON.stringify(mockOptions.defaults)), skipOptions);
      promptCase = JSON.parse(JSON.stringify(mockPrompts.defaults));
    });

    it('should generate the expected files and their content', function (done) {
      gulpAngular.run({}, function () {

[zckrs]

On Windows this test failed.

utils.js is really neccesary ?

[zckrs]

Use path.sep

[marani]

• 2 main tests files now look cleaner.
• If input path is absolute, generator will exit with error message.
• normalizePath now should work on Windows.

Used this SO answer but improved with path.sep + path.sep for Windows otherwise it won't work.

[marani]

Now back to the gulpfile / .yo-rc.json problem, below is the draft of the solution that I mentioned above - making small gulp files refer to gulpfile.js then gulpfile.js refers to .yo-rc.json.

This way people can easily put the path definition to config.paths to replace .yo-rc.json if they don't want to use .yo-rc.json anymore.

See the diff Proposed

Basically just wrap gulp.task definitions around a function (registerTasks) then export it so that gulpfile.js can pass a config argument in.

Any comment? If you all agree with this I will merge then squash again.

[Swiip]

Seams great to me.

But I don't like your last proposal. It create a strange and complex structure on the gulp files. I even prefer putting the configuration in the gulp object if you want. (Something like gulp.config = ... or gulp.paths = ..., it's not ideal but...)

In fact, I'm still puzzled about using .yo-rc.json as a config file. For other choices, it's also the choices made at the time of generation but nothing say it's maintained (why not installing UI boostrap after?). So finally, why not duplicating this configuration in the gulpfile.js ?

[marani]

gulp.config seems like a cleaner choice, the only thing is that it doesn't define dependencies explicitly. But that's fine as it's the only exception & we're not gonna unit test the gulp files anyway. It could clash with gulp api but seems like either keyword paths or config won't appear in gulp api anytime soon. So it's a good idea.

I still think duplicating is weird, imagine once we have sub-generators, suppose we need to change a path, says, appPath, we have to go to both .yo-rc.json & gulpfile.js to change. There should be a single source of truth, either gulpfile.js or .yo-rc.json.

Well but to be really honest, I also think .yo-rc.json is Not a convenient place to change things. It should just be moved away after the app is generated. The file name itself is already ugly, a meaningless name with a dot. It's true that people don't expect to change anything there. Unless it's renamed to yofile.js, or yofile.json but that's not happening soon.

However, .yo-rc.json is still a better place to interact with the sub generators, compared to gulpfile.js. The file structure in yo-rc.json is readable by the generators, in gulpfile.js it's not.

Oh well this is quite a tie.

[marani]

So the ultimate aim is to make it clear to user, about what to do if they want to change paths after the app is generated.

I think for now let's keep it in yo-rc.json:
It's clear enough when every gulp file refers to gulpfile.js to get config property.
Then we make sure that gulpfile.js is clearly reading from .yo-rc.json.
This pluses good documenting, people will easily understand that they have to go to .yo-rc.json for path change.
Then the goal is achieved.

Unless there are complaints about this (I hope not :D), then we will change it later, minor changes anyway.

So now I will implement gulp.paths then hopefully it's good to merge.

[marani]

I think I will search all popular generators & see where they store configurable path. That also affects where people expect to edit paths from.

[marani]

According to the list

generator-	path config
angular	bower.json
angular-fullstack	gruntfile.js
jhipster	gulpfile.js
gulp-webapp	n/a
mobile	n/a
wordpress	n/a
webapp	gruntfile.js
ionic	n/a
gulp-angular	🎉
chrome-extension	gruntfile.js
backbone	gruntfile.js
ember	gruntfile.js
jekyllrb	gruntfile.js
polymer	gruntfile.js

All of them also store in .yo-rc.json if there is path config. Alright so I'm gonna put settings in both places.. Can't reject the nature. Duplication for paths only is minor anyway...

[Swiip]

👍

[marani]

Still need more tests on windows. Probably needs to use slash before passing the paths further down the process. I can't install any big npm package on windows, it always breaks in the middle for some weird race condition. Trying again now.

[Swiip]

Is it ok to merge?

I've got some big refacto going on that I want to merge on this one quickly.

[marani]

Now it's good. Do you want me to change the version of slash to ~ ? Do I need to squash again ?

[marani]

Changed slash version to ~. Commits message might be helpful fo this large commit, so I wouldn't squash.

[Swiip]

Don't like either to "squash too much". Thank you!