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

Support input where there is both standard input and arguments #91

Merged
merged 4 commits into from
May 10, 2019

Conversation

taktran
Copy link
Contributor

@taktran taktran commented May 8, 2019

Rather than error when there are input repos from both standard input and arguments, just allow it and process both. Especially good for usage with xargs. Also made the arbitrary call to process standard input after arguments, to mimic xargs usage.

Main fix is in 1d14851, with other commits being clean up and refactoring steps. e1b0a68 also fixes an issue where we try to use readline when there is no standard input.

Fixes #87

To test this

stdin

echo 'Financial-Times/next-static' | ./bin/ebi.js package ''

Returns stdin result

Financial-Times/next-static

args

./bin/ebi.js package '' Financial-Times/next-static

Returns arg result

Financial-Times/next-static

stdin + args

echo 'Financial-Times/next-static' | ./bin/ebi.js package '' 'Financial-Times/next-search-page'

Returns arg result, then stdin result (like stdin + xargs + args below)

Financial-Times/next-search-page
Financial-Times/next-static

stdin + xargs

echo 'Financial-Times/next-static' | xargs ./bin/ebi.js package ''

Returns stdin result

Financial-Times/next-static

stdin + xargs + args

echo 'Financial-Times/next-static' | xargs ./bin/ebi.js package '' 'Financial-Times/next-search-page'

Returns arg result, then stdin result

Financial-Times/next-search-page
Financial-Times/next-static

Also add a `test:watch` for convenience
Allow custom `createInterface` implementation to be passed into
`setupReadline` to be able to simulate no stdin.
Concatenates the two lists and puts stdin after args
@taktran taktran requested a review from sjparkinson May 8, 2019 16:40
@taktran taktran requested a review from a team May 9, 2019 09:12
To make sure no `eslint` rules conflict with the prettier config, we have [eslint-config-prettier](https://github.com/prettier/eslint-config-prettier). This can be run with:

npm run eslint-check

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

@adambraimbridge adambraimbridge left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@jenniferemshepherd
Copy link
Contributor

THIS IS SO COOL 😎

Copy link
Contributor

@jenniferemshepherd jenniferemshepherd left a comment

Choose a reason for hiding this comment

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

Neat 👌

@taktran taktran merged commit 06460ef into master May 10, 2019
@taktran taktran deleted the bugs/xargs branch May 10, 2019 09:18
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.

Error when using xargs
3 participants