-
-
Notifications
You must be signed in to change notification settings - Fork 26.8k
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
Recursively copy the template folder #74
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,25 +35,39 @@ module.exports = function(hostPath, appName, verbose) { | |
); | ||
|
||
// Copy the files for the user | ||
function copySync(src, dest) { | ||
return fs.writeFileSync(dest, fs.readFileSync(src)); | ||
function copyFileSync(src, dest) { | ||
var target = dest; | ||
if (fs.existsSync(dest)) { | ||
if (fs.lstatSync(dest).isDirectory()) { | ||
target = path.join(dest, path.basename(src)); | ||
} | ||
} | ||
fs.writeFileSync(target, fs.readFileSync(src)); | ||
} | ||
fs.mkdirSync(path.join(hostPath, 'src')); | ||
fs.readdirSync(path.join(selfPath, 'template/src')).forEach(function(filename) { | ||
copySync( | ||
path.join(selfPath, 'template/src', filename), | ||
path.join(hostPath, 'src', filename) | ||
); | ||
}); | ||
fs.readdirSync(path.join(selfPath, 'template')).forEach(function(filename) { | ||
if (fs.lstatSync(path.join(selfPath, 'template', filename)).isDirectory()) { | ||
return | ||
} | ||
copySync( | ||
path.join(selfPath, 'template', filename), | ||
path.join(hostPath, filename) | ||
); | ||
}); | ||
|
||
function copyFolderSync(src, dest) { | ||
var files = []; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: two space indent please |
||
var targetFolder = path.join(dest, path.basename(src)).replace('/template', ''); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it's not safe to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you do that in the call site. It's confusing to have a generically named function copyFolderSync that replaces some hardcoded path. Especially since you're recursively calling it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, that makes sense. I'll find another way to do it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. path.resolve(base, '..') works cross platform |
||
if (!fs.existsSync(targetFolder)) { | ||
fs.mkdirSync(targetFolder); | ||
} | ||
if (fs.lstatSync(src).isDirectory()) { | ||
files = fs.readdirSync(src); | ||
files.forEach(function (file) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: remove space between |
||
var currentSrc = path.join(src, file); | ||
if (fs.lstatSync(currentSrc).isDirectory()) { | ||
copyFolderSync(currentSrc, targetFolder); | ||
} else { | ||
copyFileSync(currentSrc, targetFolder); | ||
} | ||
} ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: remove space between |
||
} | ||
} | ||
|
||
copyFolderSync( | ||
path.join(selfPath, 'template'), | ||
hostPath | ||
) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: missing |
||
|
||
// Run another npm install for react and react-dom | ||
console.log('Installing react and react-dom from npm...'); | ||
|
@@ -71,7 +85,7 @@ module.exports = function(hostPath, appName, verbose) { | |
|
||
// Make sure to display the right way to cd | ||
var cdpath; | ||
if (path.join(process.cwd(), appName) == hostPath) { | ||
if (path.join(process.cwd(), appName) === hostPath) { | ||
cdpath = appName; | ||
} else { | ||
cdpath = hostPath; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is never going to happen right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean the file is never going to exist? Yeah I guess not. Updating...