added torrent URL support via issue #28

max-mapper / torrent

download torrents with node from the CLI

https://www.npmjs.org/package/torrent

633 stars 78 forks source link

added torrent URL support via issue #28 #41

Closed zoo1 closed 9 years ago

zoo1 commented 9 years ago

Added torrent URL support for issue #28. My first commit to this repository so please let me know if I am committing any styling violations.

max-mapper commented 9 years ago

@zoo1 hi, thanks for the PR. a couple issues:

the style has some issues, you can run npm test to see them
to support fetching torrent files by url it means handling http://, https:// and redirects at the least. I would recommend using require('got') to handle these cases. but it begs the question: should the torrent module include an http client, or should users simply use curl or any other tool to download the files first. I personally would lean towards the latter, and keep torrent simpler.

LinusU commented 9 years ago

Or feross/simple-get, but I'm also leaning towards piping output from curl or any other program, which is already supported. If we are going to support http we probably should support proxies, show a progress-bar while downloading the file and have sensible timeouts. All of this is better handled with a separate program.

zoo1 commented 9 years ago

Thanks for the response guys, from the responses I am getting it seems like this feature is undesired. I will retract my pull request.

LinusU commented 9 years ago

We appreciate your time and effort, but I think that this is best solved outside of this module. Thank you!

zoo1 commented 9 years ago

It might be wise to close issue #28 to where no one repeats the same mistake I just made.

LinusU commented 9 years ago

Very wise, thank you for the reminder :sun_with_face: