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

Expose the directory where the generator is being executed #982

Merged
merged 1 commit into from Dec 1, 2016
Merged

Expose the directory where the generator is being executed #982

merged 1 commit into from Dec 1, 2016

Conversation

ericmorand
Copy link

@ericmorand ericmorand commented Nov 27, 2016

This PR propose an elegant solution to issue #947.

By exposing the directory where the generator was executed, we give the opportunity to subgenerators to scaffold into the current folder instead of the project folder.

Tests have been implemented. I hope the name contextRoot for the property is OK to everyone.

@ericmorand
Copy link
Author

Can someone relaunch the checks? There was a timeout on one of them that has nothing to do with that PR.

@mischah
Copy link
Member

mischah commented Nov 27, 2016

Restarted Node 6.

@ericmorand
Copy link
Author

@mischah Thanks a lot!

@sindresorhus
Copy link
Member

Please give the pull request a descriptive title ;)

@ericmorand ericmorand changed the title Fix issue #947 Expose the directory where the generator is being executed Nov 28, 2016
@ericmorand
Copy link
Author

ericmorand commented Nov 28, 2016

Done. I also clarified the description.

@SBoudrias
Copy link
Member

So, this.env.cwd is always available (it doesn't change when the destinationRoot changes). Any reason you want to also make it available as this.contextRoot?

@ericmorand
Copy link
Author

ericmorand commented Nov 30, 2016

this.env.cwd does change when destinationRoot() is called:

Base.prototype.destinationRoot = function (rootPath) {
  if (_.isString(rootPath)) {
    this._destinationRoot = path.resolve(rootPath);

    if (!pathExists.sync(rootPath)) {
      mkdirp.sync(rootPath);
    }

    process.chdir(rootPath);
    this.env.cwd = rootPath;

    // Reset the storage
    this.config = this._getStorage();
  }

  return this._destinationRoot || this.env.cwd;
};

That's exactly the point of the issue #947 .

@ericmorand
Copy link
Author

To go further on that point, I didn't want to change the behavior of destinationRoot() regarding this.env.cwd because I'm pretty sure a lot of already available generators are written with that behavior in mind - when destinationRoot() is called, this.env.cwd is changed.

That's why I did add a new property to store the initial value of this.env.cwd.

@SBoudrias
Copy link
Member

Thanks for the answer. I missed that line reading through the code yesterday. Sounds good to merge :)

@SBoudrias SBoudrias merged commit f6f4dda into yeoman:master Dec 1, 2016
@ericmorand ericmorand deleted the issue_947 branch December 1, 2016 09:24
@ericmorand
Copy link
Author

I'm not sure about how to update the documentation. Should I create an issueand PR now on yeoman/yeoman.github.io or should I wait for the version that contains that PR to be released?

@mischah
Copy link
Member

mischah commented Dec 1, 2016

Hej @ericmorand,

open now and reference yeoman/yeoman.io#700

Thanks for your help :octocat:

@mischah
Copy link
Member

mischah commented Dec 1, 2016

Even better:
Create PR against the branch doc/yeoman-generator/1.0.0 instead of against the default source branch 😘

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.

None yet

4 participants