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

fix-#64 - Add option to avoid new folder #69

Merged
merged 2 commits into from
Oct 4, 2022
Merged

fix-#64 - Add option to avoid new folder #69

merged 2 commits into from
Oct 4, 2022

Conversation

xnivaxhzne
Copy link

Added an extra option -f or --flat to create the files in the same folder without creating new folder in the name of the component.

This will fix the feature request of #64

Copy link
Owner

@arminbro arminbro left a comment

Choose a reason for hiding this comment

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

Hey @kavinkuma6, thanks for taking the time to contribute to GRC! 👏

This pr looks good, just a few minor changes. First, we can remove all the extra logic around checkIsFlat as it is unnecessary. We can use cmd.flat directly within the generateComponentUtils.js file, and it will automatically handle the -f alias case.

Once those changes are in, I would be more than happy to merge your PR. 😃

@@ -2,10 +2,12 @@ const {
generateComponent,
getComponentByType,
getCorrespondingComponentFileTypes,
checkIsFlat,
Copy link
Owner

Choose a reason for hiding this comment

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

You can remove the checkIsFlat import. This function will not be needed as we can use cmd.flat directly within the generateComponentUtils.js file, and it will handle the -f alias for us automatically.

Suggested change
checkIsFlat,

} = require('../utils/generateComponentUtils');

function initGenerateComponentCommand(args, cliConfigFile, program) {
const selectedComponentType = getComponentByType(args, cliConfigFile);
const isFlat = checkIsFlat(args);
Copy link
Owner

Choose a reason for hiding this comment

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

You can remove this line for the reason mentioned above.

Suggested change
const isFlat = checkIsFlat(args);

@@ -37,7 +40,7 @@ function initGenerateComponentCommand(args, cliConfigFile, program) {
// Component command action.

componentCommand.action((componentNames, cmd) =>
componentNames.forEach((componentName) => generateComponent(componentName, cmd, cliConfigFile))
componentNames.forEach((componentName) => generateComponent(componentName, cmd, cliConfigFile, isFlat))
Copy link
Owner

Choose a reason for hiding this comment

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

We don't need to pass isFlat to generateComponent for the reason mentioned above.

Suggested change
componentNames.forEach((componentName) => generateComponent(componentName, cmd, cliConfigFile, isFlat))
componentNames.forEach((componentName) => generateComponent(componentName, cmd, cliConfigFile))

@@ -74,7 +74,7 @@ Please make sure you're pointing to the right custom template path in your gener
}
}

function componentTemplateGenerator({ cmd, componentName, cliConfigFile }) {
function componentTemplateGenerator({ cmd, componentName, cliConfigFile, isFlat }) {
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
function componentTemplateGenerator({ cmd, componentName, cliConfigFile, isFlat }) {
function componentTemplateGenerator({ cmd, componentName, cliConfigFile }) {

@@ -127,13 +127,13 @@ function componentTemplateGenerator({ cmd, componentName, cliConfigFile }) {
}

return {
componentPath: `${cmd.path}/${componentName}/${filename}`,
componentPath: `${cmd.path}${isFlat ? '' : `/${componentName}`}/${filename}`,
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
componentPath: `${cmd.path}${isFlat ? '' : `/${componentName}`}/${filename}`,
componentPath: `${cmd.path}${cmd.flat ? '' : `/${componentName}`}/${filename}`,

@@ -303,7 +303,7 @@ Please make sure you're pointing to the right custom template path in your gener
filename = customTemplateFilename;

return {
componentPath: `${cmd.path}/${componentName}/${filename}`,
componentPath: `${cmd.path}${isFlat ? '' : `/${componentName}`}/${filename}`,
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
componentPath: `${cmd.path}${isFlat ? '' : `/${componentName}`}/${filename}`,
componentPath: `${cmd.path}${cmd.flat ? '' : `/${componentName}`}/${filename}`,

@@ -329,7 +329,7 @@ const componentTemplateGeneratorMap = {
[buildInComponentFileTypes.LAZY]: componentLazyTemplateGenerator,
};

function generateComponent(componentName, cmd, cliConfigFile) {
function generateComponent(componentName, cmd, cliConfigFile, isFlat) {
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
function generateComponent(componentName, cmd, cliConfigFile, isFlat) {
function generateComponent(componentName, cmd, cliConfigFile) {

@@ -347,6 +347,7 @@ function generateComponent(componentName, cmd, cliConfigFile) {
componentName,
cliConfigFile,
componentFileType,
isFlat,
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
isFlat,

Comment on lines 432 to 435
function checkIsFlat(args) {
return args.find((arg) => arg.includes('-f') || arg.includes('--flat'));
}

Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
function checkIsFlat(args) {
return args.find((arg) => arg.includes('-f') || arg.includes('--flat'));
}

module.exports = {
generateComponent,
getComponentByType,
getCorrespondingComponentFileTypes,
checkIsFlat,
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
checkIsFlat,

@xnivaxhzne
Copy link
Author

hey @arminbro, I have done the changes as per your suggestions. Please review them.
Thank you..

Copy link
Owner

@arminbro arminbro left a comment

Choose a reason for hiding this comment

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

This looks good. Thanks again for contributing, @kavinkuma6.

@arminbro arminbro merged commit 578aea6 into arminbro:main Oct 4, 2022
@github-actions
Copy link

github-actions bot commented Oct 4, 2022

🎉 This PR is included in version 7.4.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants