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

Detect gsed and use it if it exists #25

Merged
merged 2 commits into from Feb 10, 2016
Merged

Conversation

anshul
Copy link
Contributor

@anshul anshul commented Feb 10, 2016

No description provided.

@anshul
Copy link
Contributor Author

anshul commented Feb 10, 2016

@stevemao Squashed the changes into a single commit.

@stevemao
Copy link
Member

Is it common to symlink gsed to sed? homebrew does it. Maybe we should instead warn the user?

@anshul
Copy link
Contributor Author

anshul commented Feb 10, 2016

This is such a common pattern on osx/homebrew that it's pretty muh universal on most osx installs. brew install coreutils findutils gnu-sed will create gsed, gfind, greadlink, etc. I don't like the idea of overriding the apple implementations of these commands using --with-default-names.

@anshul
Copy link
Contributor Author

anshul commented Feb 10, 2016

Sorry, I read your comment wrong. Edited mine. I think, everyone is served better if npm install just works out of the box on a typical osx install. Recommending overriding the apple sed command could leave other commands that depend on the apple installation in a bad shape.

@anshul
Copy link
Contributor Author

anshul commented Feb 10, 2016

Looks like ports also installs a gsed. http://apple.stackexchange.com/a/166593

@stevemao
Copy link
Member

I'm fine with this. I'll see what other people think.

@paulirish
Copy link
Member

Yup. This is good. Osx sed is crap but some people won't want to replace it and that's fine.

SED=sed
if hash gsed 2>/dev/null; then
SED=gsed
fi
Copy link
Member

Choose a reason for hiding this comment

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

Can you make this a one liner?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure

@anshul
Copy link
Contributor Author

anshul commented Feb 10, 2016

@paulirish Does that one liner look fine?

@paulirish
Copy link
Member

Yup! Ace.

paulirish added a commit that referenced this pull request Feb 10, 2016
Detect gsed and use it if it exists
@paulirish paulirish merged commit 1544df3 into so-fancy:master Feb 10, 2016
@stevemao stevemao mentioned this pull request Feb 11, 2016
OJFord added a commit to OJFord/homebrew that referenced this pull request Feb 12, 2016
Merged PR so-fancy/diff-so-fancy#25 prefers `gsed` over `sed`, so
no need to replace ocurrences to `gsed` here.
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.

None yet

3 participants