Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Add a CLI #180

Merged
merged 3 commits into from
Oct 24, 2018
Merged

Add a CLI #180

merged 3 commits into from
Oct 24, 2018

Conversation

nono
Copy link
Contributor

@nono nono commented Oct 24, 2018

Description of the Change

The CLI can be used like this:

$ watcher path/to/watch

Alternate Designs

I've tried to do something simple, with no external dependencies. An alternative would be to copy chokidar-cli, with more options. But if we want to go in this direction, it'd better to do that outside of this repository, and in a new npm package.

Benefits

It should help for testing @atom/watcher and reporting issues.

Possible Drawbacks

It will be one more file in the npm package, and the script may have to be maintained in the future.

Applicable Issues

N/A

Copy link
Contributor

@smashwilson smashwilson left a comment

Choose a reason for hiding this comment

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

Our linter isn't happy with this. We're using standard.js for the JavaScript side of things, so you should be able to run npm run format:js and get most of the way there.

Once that's green i'm 💯 on merging this, this would be super handy!

lib/cli.js Outdated
}

function start(dirs, verbose) {
const options = { recursive: true };
Copy link
Contributor

Choose a reason for hiding this comment

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

Were you meaning to make this another user-controlled flag?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I think that recursive is probably the wanted option here. Do you think I should use an option here?

@smashwilson
Copy link
Contributor

Oh, one more thing - would you mind adding a section to the README about this as well?

The CLI can be used like this:

    $ watcher path/to/watch

It should help for testing @atom/watcher and reporting issues.
@nono
Copy link
Contributor Author

nono commented Oct 24, 2018

I've fixed the lint (I've used prettier instead of standard the first time), and I've added a paragraph in the README. I think it's ready to be merged if the CI is Okay.

@smashwilson
Copy link
Contributor

🎉

@smashwilson smashwilson merged commit cbbe53e into atom:master Oct 24, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants