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

[WIP] Feature/blueprint clone command #133

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions src/cli/cmds/clone.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import buildBlueprintCommands from './clone/build-blueprint-commands';

const usage = `Usage:
$0 clone <blueprint> <name>
$0 help clone <blueprint>`;

module.exports = {
command: 'clone <blueprint> <name>',
aliases: ['g', 'gen'],
Copy link
Collaborator

Choose a reason for hiding this comment

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

probably the wrong aliases

Copy link
Contributor

Choose a reason for hiding this comment

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

indeed... aliases are optional so you can set an empty array or remove the key entirely.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks!

I've fixed it.

describe: 'Clone a blueprint with a different name',
builder: yargs => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We'll want all commands to follow the same pattern, I'm working to finalize the api for that, but take a look at #132 for the current thinking. @jamesr73 ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll take a closer look at this before providing final feedback... this seems to fall between the pattern used for simple yargs commands like init and the more involved pattern used to support the blueprint specific generate commands.

yargs
.usage(usage)
.option('dry-run', {
alias: 'd',
describe: "List files but don't generate them",
Copy link
Collaborator

@anithri anithri Oct 29, 2017

Choose a reason for hiding this comment

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

2 different quoting styles used in this line and last/next. If you install and run prettier, that will help with your formatting.

This is the only descepency that eslint finds is in switch statements. Prettier wants cases indented, and eslint wants them flush with the switch statement.

// NO
export default function(state = defaultState, action) {
  switch (action.type) {
    default:
      return state;
  }
}
// YES
export default function(state = defaultState, action) {
  switch (action.type) {
  default:
    return state;
  }
}

And so I've been editing it back after i yarn run pretty

Copy link
Contributor

Choose a reason for hiding this comment

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

@anithri

Prettier would use the two different quoting styles, because the describe string has an embedded single quote. Eslint should already allow that (and complain if double quotes are used in any other circumstance). The rule for quoting was modified by a recent PR

"quotes": [2, "single", {"avoidEscape": true}],

I prefer the prettier formatting for switch, and not having any manual step to undo it! This can also be accommodated by eslint:

"indent": [2, 2, {"SwitchCase": 1}],

Suggest we make that change in .eslintrc

type: 'boolean'
})
.option('verbose', {
alias: 'v',
describe: 'Verbose output, including file contents',
type: 'boolean'
})
.group(['dry-run', 'verbose', 'help'], 'Generate Options:')
.updateStrings({
'Commands:': 'Blueprints:',
'Options:': 'Blueprint Options:'
});
return buildBlueprintCommands().reduce(
(yargs, command) => yargs.command(command),
yargs
);
},
handler: argv => console.error(`Unrecognised blueprint '${argv.blueprint}'`)
};
96 changes: 96 additions & 0 deletions src/cli/cmds/clone/build-blueprint-command.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
/*
Build a yargs command module object from options defined in the blueprint
https://github.com/yargs/yargs/blob/master/docs/advanced.md#providing-a-command-module

Target object structure:
{
command: 'blueprint <name>',
aliases: [],
describe: 'Generates a blueprint',
builder: yargs => yargs,
handler: argv => runner.run()
}
*/
const buildBlueprintCommand = (blueprint, runner) => {
// extract custom command pieces
let {
aliases = [],
options,
examples,
epilog,
epilogue,
check,
sanitize
} = blueprint.command;

// mandate the command name to guarantee name is passed to generate task
let command = `${blueprint.name} <name>`;

// rc aliases override blueprint configuration
aliases = [].concat(blueprint.settings.aliases || aliases);

// default usage
let usage = `Usage:\n $0 clone ${command}`;
aliases.forEach(
alias =>
(usage += `\n $0 clones ${command.replace(blueprint.name, alias)}`)
);

// default options from settings
if (options && blueprint.settings) {
Object.keys(options).forEach(option => {
if (blueprint.settings[option]) {
options[option].default = blueprint.settings[option];
}
});
}

// alterate epilogue keys
epilogue = epilogue || epilog;

// builder brings together multiple customizations, whilst keeping the
// options easy to parse for prompting in the init command
const builder = yargs => {
yargs.usage(usage).strict(false); // allow undocumented options through

if (options) yargs.options(options);
if (check) yargs.check(check, false);
if (examples) {
[].concat(examples).forEach(example => yargs.example(example));
}
if (epilogue) yargs.epilogue(epilogue);

return yargs;
};

// handler runs the generate blueprint task
const handler = argv => {
// merge command line options into rc options
let options = { ...blueprint.settings, ...argv };

// tidy up options before passing them on so that all hooks have access
// to the clean version
if (sanitize) options = sanitize(options);

const cliArgs = {
entity: {
name: argv.name,
options,
rawArgs: argv
},
debug: argv.verbose || false,
dryRun: argv.dryRun || false
};
runner.run(blueprint.name, cliArgs);
};

return {
command,
aliases,
describe: `Clones ${blueprint.name}`,
builder,
handler
};
};

export default buildBlueprintCommand;
26 changes: 26 additions & 0 deletions src/cli/cmds/clone/build-blueprint-commands.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import _merge from 'lodash/merge';
import _cloneDeep from 'lodash/cloneDeep';

import getEnvironment from '../../environment';
import Clone from '../../../sub-commands/clone';
import buildBlueprintCommand from './build-blueprint-command';

const loadBlueprintSettings = (blueprint, bp) =>
(blueprint.settings = _merge(
_cloneDeep(bp.common),
_cloneDeep(bp[blueprint.name])
));

const buildBlueprintCommands = () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

All of the blueprints will already be found, read, and have it's settings already merged. Again the api is not quite ready yet, but soon. And any suggestions would be most welcome. see #132 for current thinking

const environment = getEnvironment();
const subCommand = new Clone(environment);

const { blueprints, settings: { bp = {} } } = environment.settings;

return blueprints.generators().map(blueprint => {
loadBlueprintSettings(blueprint, bp);
return buildBlueprintCommand(blueprint, subCommand);
});
};

export default buildBlueprintCommands;
84 changes: 84 additions & 0 deletions src/models/blueprint-cloner.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
import path from 'path';
import { copySync } from 'fs-extra';
import walkSync from 'walk-sync';

import { fileExists } from '../util/fs';

const FILES_BLACKLIST = ['.ds_store', '.git', '.gitkeep'];
Copy link
Collaborator

Choose a reason for hiding this comment

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

We definitely need a FILES_BLACKLIST. I'm just wondering if there's value to making it an rc option that would include what you have here as defaults.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It might be worth it 🤔

If we set the defaults it's always good to let the user add more customization.

But in what other circumstances will we need to blacklist files besides clone?


export default class BlueprintCloner {
constructor(blueprint, options) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

let's use constructor(blueprint, options = {}) instead of line 12

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

this.blueprint = blueprint;
this.options = options || {};
this.ui = options.ui;
}

clone() {
const blueprint = this.blueprint;
Copy link
Collaborator

Choose a reason for hiding this comment

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

By the time a blueprint is added to the collection, it will already have a validated existent path to itself. And will also have a method that returns it's files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Awesome!

I'll make the changes once the new api is ready.

this.ui.writeInfo('cloning blueprint...');

const cloneToDirectory = this.cloneToDirectory();
Copy link
Collaborator

Choose a reason for hiding this comment

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

The directory we clone to will be determined at the settings level, something like rc.get('path.bpTarget). So by this point in execution, you'll either have a falsy value or a validated, existing path you can write to. Falsy values should generate a warning or an error.

It seems to me that making assumptions based on the search path order, is risking unintended consequences. Better to get 'permission' with an explicity defined path.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I totally agree, it is risky to make assumptions based on the search path order and it does make sense to get that permission from the explicit path.

I was aware that I will need to change the cloneDirectory to something like rc.get('path.bpTarget').

I will make the change once the new rc api is ready.

this.ui.writeInfo('cloning into: ' + cloneToDirectory);

const blueprintFiles = this.blueprintFiles();
this.cloneFiles(blueprint.path, cloneToDirectory, blueprintFiles);
}

cloneFiles(sourceDirectory, cloneToDirectory, files) {
files.forEach((file) => {
let sourcePath = path.resolve(sourceDirectory, file);
let destinationPath = path.resolve(cloneToDirectory, file);
this.cloneFile(sourcePath, destinationPath);
});
}

cloneFile(sourcePath, destinationPath) {
const ui = this.ui;
const dryRun = this.options.dryRun;

ui.writeDebug(`Attempting to clone file: ${destinationPath}`);
if (fileExists(destinationPath)) {
ui.writeError(
`Not writing file. File already exists at: ${destinationPath}`
);
} else {
if (!dryRun) {
copySync(sourcePath, destinationPath);
ui.writeCreate(destinationPath);
} else {
ui.writeWouldCreate(destinationPath);
}
}
}

cloneToDirectory() {
const settings = this.options.settings;

// settings.blueprints.searchPaths[0] will be settings.cloneTo
return path.resolve(settings.blueprints.searchPaths[0], this.newBlueprintName());
}

newBlueprintName() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is another thing that should be from rc, so that we can add --name <newName> or --cloneTo or similar.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a great idea.

Let me check if I am understanding correct, rc will have:

  • All the settings from the .blueprintrc.
  • The UI environment to write.
  • The arguments sent from the CLI.

const options = this.options;

if(options.entity) {
return options.entity.name;
}
}

blueprintFiles() {
const blueprint = this.blueprint;
let blueprintFiles = walkSync(blueprint.path, { directories: false, ignore: FILES_BLACKLIST });
blueprintFiles = this.filterBlacklistedFiles(blueprintFiles);
return blueprintFiles;
}

filterBlacklistedFiles(files) {
return files.filter((file) => !this.isBlacklistedFile(file));
}

isBlacklistedFile(file) {
const fileName = path.basename(file).toLowerCase();
return FILES_BLACKLIST.includes(fileName);
}
}
26 changes: 26 additions & 0 deletions src/sub-commands/clone.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import SubCommand from '../models/sub-command';
import Blueprint from '../models/blueprint';
import CloneBlueprint from '../tasks/clone-blueprint';
import chalk from 'chalk';

// Primary purpose is to take cli args and pass them through
// to the proper task that will do the generation.
//
// Logic for displaying all blueprints and what their options
// are will live in here. For now it's pretty baren.
class Clone extends SubCommand {
constructor(options) {
super(options);
this.cloneTask = new CloneBlueprint(this.environment);
}

run(blueprintName, cliArgs) {
if (cliArgs.debug) {
this.ui.setWriteLevel('DEBUG');
}

this.cloneTask.run(blueprintName, cliArgs);
}
}

export default Clone;
32 changes: 32 additions & 0 deletions src/tasks/clone-blueprint.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import Task from '../models/task';
import BlueprintCloner from '../models/blueprint-cloner';

export default class extends Task {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Lets add an explicit class name, if only to make our IDE's happier.

Copy link
Contributor

Choose a reason for hiding this comment

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

FYI... none of the other Tasks currently specify a class name.

Copy link
Collaborator

Choose a reason for hiding this comment

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

well, that's true. AirBNB recommend the existing way too:
https://github.com/airbnb/javascript#modules--prefer-default-export

TIL

constructor(environment) {
super(environment);
}

run(blueprintName, cliArgs) {
const blueprint = this.lookupBlueprint(blueprintName);

const entity = {
name: cliArgs.entity.name,
options: cliArgs.entity.options
};

const blueprintOptions = {
originalBlueprintName: blueprintName,
ui: this.ui,
settings: this.settings,
dryRun: cliArgs.dryRun,
entity
};

const blueprintCloner = new BlueprintCloner(blueprint, blueprintOptions);
blueprintCloner.clone();
}

lookupBlueprint(name) {
return this.settings.blueprints.lookup(name);
}
}