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

Adding flags to use implicit defaults and wildcard folders #55

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

toddtarsi
Copy link

@toddtarsi toddtarsi commented Nov 26, 2019

I found an issue in the recurse logic. This tool creates a bunch of named exports. It can recurse. It grabs default exports normally. This means it has issues recursing itself because it's not grabbing the child folder's named exports. It's resolved by using wildcard characters on folders. I don't want to mess with existing stuff, so it's an optional flag.

Also, I added a flag so defaults could be made implicit instead of explicit. More OCD than anything.

Thanks for the great tool and if you don't want to incorporate this stuff, let me know. Thanks!

Also, I can change the file permissions and gitignore back np.

@gajus
Copy link
Owner

gajus commented Nov 26, 2019

Also, I can change the file permissions and gitignore back np.

Yes, please.

@gajus
Copy link
Owner

gajus commented Nov 26, 2019

The current PR is a mess. It would probably be easier to force push just the changes.

@toddtarsi
Copy link
Author

toddtarsi commented Nov 26, 2019

@gajus - Done and done. Also, I rebased all the 644 commits down to one. Sorry about that!

Copy link
Owner

@gajus gajus left a comment

Choose a reason for hiding this comment

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

.gitattributes Outdated Show resolved Hide resolved
@@ -97,6 +97,14 @@ Options:
First extension will always be preferred to homonyms
with another allowed extension.
[array] [default: ["js"]]
--implicitDefault, -m Uses defaults as implicit instead of named.
Copy link
Owner

Choose a reason for hiding this comment

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

Please use dashes for CLI program parameters, i.e. --implicit-default.

Copy link
Author

@toddtarsi toddtarsi Nov 26, 2019

Choose a reason for hiding this comment

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

@gajus - Are you sure? I was following the convention outlined in ignoreDirectories and ignoreUnsafe.

Copy link
Owner

Choose a reason for hiding this comment

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

Right. That should not have been the case originally. Please keep the convention. I will release a breaking change later.

Copy link
Author

Choose a reason for hiding this comment

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

@gajus - Sounds good. If everything else is okay, I'll make some tests over the next couple days to check for any funny business between the various flags.

Copy link

Choose a reason for hiding this comment

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

What is the status of this PR? I would love this change.

README.md Outdated Show resolved Hide resolved
create-index Outdated Show resolved Hide resolved
@toddtarsi
Copy link
Author

@gajus - Thanks for reviewing this. I added everything but the hyphen delimiters and the tests. I'll look at the tests a little later. If you still want the hyphen delimiters, I can do that as well, but I saw an existing convention further back in the repo.

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.

2 participants