Skip to content

feat(sed): support multiple file names - #314

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

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

Conversation

@nfischer

@nfischer nfischer commented Jan 26, 2016

Copy link
Copy Markdown
Member

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
Comment thread src/sed.js Outdated

@ariporad ariporad Jan 27, 2016

Copy link
Copy Markdown
Contributor

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"?

@nfischer nfischer Jan 27, 2016

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done

@ariporad

ariporad commented Jan 27, 2016

Copy link
Copy Markdown
Contributor

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

@nfischer

nfischer commented Jan 27, 2016

Copy link
Copy Markdown
Member Author

@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

ariporad commented Jan 28, 2016

Copy link
Copy Markdown
Contributor

LGTM!

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

nfischer commented Jan 28, 2016

Copy link
Copy Markdown
Member Author

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

cvrebert commented Jan 28, 2016

Copy link
Copy Markdown

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

@nfischer

nfischer commented Jan 28, 2016

Copy link
Copy Markdown
Member Author

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

cvrebert commented Jan 28, 2016

Copy link
Copy Markdown

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