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

data URI support #6

Closed
wants to merge 2 commits into from
Closed

data URI support #6

wants to merge 2 commits into from

Conversation

sonnyp
Copy link
Contributor

@sonnyp sonnyp commented Jan 22, 2016

this PR includes commit for stdin support #5 as well

@@ -2,10 +2,15 @@
'use strict';
const meow = require('meow');
const opn = require('opn');
const getStdin = require('get-stdin');
const tempWrite = require('temp-write');
const dataUriToBuffer = require('data-uri-to-buffer');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe require those when needed to reduce startup time ?

Copy link
Owner

Choose a reason for hiding this comment

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

Premature optimization is the root of all evil.

http://c2.com/cgi/wiki?PrematureOptimization

A few requires have no noticeable impact.

@sindresorhus
Copy link
Owner

I think you misunderstood me. There's no need for data URI support. It can just be stdin.

opn data:,Hello%2C%20World!

Could just be

echo 'Hello World!' | opn

Note, not URL, but the content as stdin.

@sonnyp
Copy link
Contributor Author

sonnyp commented Jan 27, 2016

I think you misunderstood me.

No

There's no need for data URI support.

That was my original request see sindresorhus/open#24

It can just be stdin.

Yes and the PR is at #5

@sindresorhus
Copy link
Owner

Yes and the PR is at #5

No, that PR adds support for reading URL from stdin. That isn't very useful as you can already do that with opn <url>. Instead, I asked for that it should read the data from stdin, save it to a tempfile and then open it.

@sonnyp
Copy link
Contributor Author

sonnyp commented Jan 27, 2016

@sindresorhus right my bad, understood

@sindresorhus
Copy link
Owner

It will probably need an --ext flag too to indicate the filetype.

We could default to .txt for text and detect binary with https://github.com/sindresorhus/file-type, so it would be automatic in most cases.

@sindresorhus
Copy link
Owner

Closing in favor of #7.

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.

2 participants