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.

Sign up for GitHub

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

Jump to bottom

Add feature ignorefile #94

Merged

blakeembrey merged 21 commits into Qard:master from Oak-V:feature_ignorefile

Sep 10, 2019

Contributor

Oak-V commented Sep 8, 2019

This implements the feature described in #93.

Usage:

# custom path
$ onchange '*/*.js' -e './demo.js' --ignore-path ./.gitignore -- prettier --write {{changed}}

# or get ./.onchangeignore
$ onchange '*/*.js' -e './demo.js' -- prettier --write {{changed}}

any questions or refactoring needs just talk.

=]

Vinícius Carvalho added 3 commits

September 8, 2019 15:30


          add flag --ignore-path and default value ./.onchangeignore

6ef20a9


          implements merge betwen excludes and ignore

3f76408


          change version from 6.0.0 to 6.1.0 due to the insertion of the new fe…

bb9e0d4

…ature --ignore-path

blakeembrey reviewed

View reviewed changes

index.js Outdated Show resolved Hide resolved

blakeembrey reviewed

View reviewed changes

package.json Outdated Show resolved Hide resolved

Vinícius Carvalho added 2 commits

September 8, 2019 17:35


          Revert "change version from 6.0.0 to 6.1.0 due to the insertion of th…

3fd224e

…e new feature --ignore-path"

This reverts commit bb9e0d4.


          move getgetIgnoreMergedFromIgnoreFile function to upper context

28b201a

blakeembrey reviewed

View reviewed changes

index.js Outdated Show resolved Hide resolved

blakeembrey reviewed

View reviewed changes

index.js Outdated Show resolved Hide resolved

blakeembrey reviewed

View reviewed changes

index.js Outdated Show resolved Hide resolved

Vinícius Carvalho added 5 commits

September 8, 2019 18:05


          fix regexp for support CRLF and LF

4861f13


          change ignorepath to disable

94d5999


          move logic to cli interface

59bc805


          remove log

46f63f4


          remove unnecessary conditional for existing file

10241c5

Contributor Author

Oak-V commented Sep 8, 2019

I see you have a lot of 'var' instead of 'const / let' would it be nice to change?

Vinícius Carvalho added 2 commits

September 8, 2019 19:11


          add default filepath and ignoring invalid value

9f76a41


          change the name function for clarity and change 'var' to 'const/let' …

c6fcee7

…syntax

Collaborator

blakeembrey commented Sep 8, 2019 •

edited

Loading

@vinyfc93 No, that should be a separate feature. Stick with var for now.

Edit: I see you already did it, I suppose you can leave it anyway now 🤷‍♂ In the future it'd be a lot easier to review if you minimize the number of things you change in a single PR, otherwise it can get frustrating for the both of us as I give feedback for things out of scope.

blakeembrey and others added 6 commits

September 8, 2019 15:44


          Fix whitespace change

af180c1


          Revert "add default filepath and ignoring invalid value"

b675caf

This reverts commit 9f76a41.


          Merge branch 'feature_ignorefile' of github.com:vinyfc93/onchange int…

bf555b1

…o feature_ignorefile


          change custom approach to node-ignore

71accd9


          remove test ignore and var

aa866fb


          fix wrong name var

1e90668

Contributor Author

Oak-V commented Sep 9, 2019

Sorry for the confusion with var and const/let. I have been working alone for a long time. Anyway, I made the changes so that we use ignore instead of a custom solution.
You had reason to pass the function to chokidar, had not realized that possibility.
I hope we solved everything.
=]

Qard approved these changes

View reviewed changes

Vinícius Carvalho added 3 commits

September 9, 2019 13:59


          Move process.cwd to upper context

1a4941f


          Change to use argv.cwd

2f97103


          Update .gitignore

a6d72c5

Oak-V requested a review from blakeembrey

September 9, 2019 17:19

Collaborator

blakeembrey commented Sep 10, 2019

I'll merge it as is but refactor a little before pushing it out - I'm not 100% sure if ignore behaves the same as the existing exclude so I'll play it safe and then re-introduce using ignore 100% for the next version.

blakeembrey merged commit 9388c2e into Qard:master

Contributor Author

Oak-V commented Sep 10, 2019

What are the next steps? How can I help you?

Oak-V deleted the feature_ignorefile branch

September 10, 2019 01:40

Oak-V restored the feature_ignorefile branch

September 10, 2019 01:41

Oak-V deleted the feature_ignorefile branch

September 10, 2019 01:41

Collaborator

blakeembrey commented Sep 10, 2019

@vinyfc93 I pushed a small tidy up in 547651c. I will document re-enabling the default value and renaming to ignore-path in the next major release. For now I'm sticking with --exclude to match the existing flag. I'll try to get a major release out soon with a couple of other fixes I'd like to see, including merging cross-spawn.

Contributor Author

Oak-V commented Sep 10, 2019

Nice! If you want my help with something else just talks.

Good job and see you soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet