Skip to content

feat(sed): support multiple file names#314

Merged
nfischer merged 1 commit intomasterfrom
sed-multiple-files
Jan 28, 2016
Merged

feat(sed): support multiple file names#314
nfischer merged 1 commit intomasterfrom
sed-multiple-files

Conversation

@nfischer
Copy link
Copy Markdown
Member

@nfischer nfischer commented Jan 26, 2016

fixes #231. Semantics are like unix sed.

Also, if we merge this, it makes the PR to add support for pipes slightly nicer.

@nfischer nfischer added this to the v0.6.0 milestone Jan 26, 2016
@nfischer nfischer mentioned this pull request Jan 26, 2016
Closed
ariporad
ariporad reviewed Jan 27, 2016
View reviewed changes
src/sed.js Outdated
Copy link
Copy Markdown
Contributor

@ariporad ariporad Jan 27, 2016

Choose a reason for hiding this comment

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

Small nitpick: maybe make this "no files given"?

Copy link
Copy Markdown
Member Author

@nfischer nfischer Jan 27, 2016

Choose a reason for hiding this comment

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

Done

@ariporad
Copy link
Copy Markdown
Contributor

ariporad commented Jan 27, 2016

I have a few notes, but it otherwise looks good.

@nfischer
Copy link
Copy Markdown
Member Author

nfischer commented Jan 27, 2016

@ariporad I addressed your comments. I also added an invalid test case, since I realized I forgot to add one of those 👍

And I removed the leading slash from one of the existing test cases to play better with Windows compatibility (not sure if it was an issue, but it should at least make the test more robust, since it doesn't depend on the user's root directory contents anymore).

@ariporad
Copy link
Copy Markdown
Contributor

ariporad commented Jan 28, 2016

LGTM!

@nfischer
feat(sed): support multiple file names
bccf620 
fixes #231. Semantics are like unix sed.
@nfischer
Copy link
Copy Markdown
Member Author

nfischer commented Jan 28, 2016

LGTM. Merging

nfischer added a commit that referenced this pull request Jan 28, 2016
@nfischer
Merge pull request #314 from shelljs/sed-multiple-files
c072738 
feat(sed): support multiple file names
@nfischer nfischer merged commit c072738 into master Jan 28, 2016
@nfischer nfischer deleted the sed-multiple-files branch January 28, 2016 03:24
@cvrebert
Copy link
Copy Markdown

cvrebert commented Jan 28, 2016

Superb! Is there any rough time horizon for the next release version of shelljs?

@nfischer
Copy link
Copy Markdown
Member Author

nfischer commented Jan 28, 2016

We've started tracking issues under the v0.6 milestone (that'll be our next release). Feel free to help out if you're available (or report any major bugs you think should be in the next release)

https://github.com/shelljs/shelljs/milestones/v0.6.0

@cvrebert
Copy link
Copy Markdown

cvrebert commented Jan 28, 2016

Thanks

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

Reviewers

No reviews

Assignees

@ariporad ariporad

Labels

feature medium priority

Projects

None yet

Milestone

v0.6.0

Development

Successfully merging this pull request may close these issues.

sed() should accept multiple file arguments

3 participants

@nfischer @ariporad @cvrebert