jackfranklin / pulldown

The minimal JavaScript package manager.
175 stars 7 forks source link

Use Node module instead of wget for downloading #2

Closed jackfranklin closed 12 years ago

jackfranklin commented 12 years ago

If we can save the external dependency that would be awesome.

@kouphax what do you think?

kouphax commented 12 years ago

Your wget method would become something like this....

wget: function(fileUrl, output, cb) {
  //default to the filename on the server if one is not passed in
  output = output || url.parse(fileUrl).pathname.split('/').pop();

  var http = require('http');
  var fs = require('fs');
  var file = fs.createWriteStream(output);

  http.get(fileUrl, function(res){
    res.on('data', function(data) {
        file.write(data);
    }).on('end', function() {
        file.end();
        cb && cb();
    });
  });
},

Seems to work but not exhaustively tested (need to check with https calls etc). I also think this would allow more flexibility around the zip file stuff as well.

jackfranklin commented 12 years ago

Awesome, thanks. Will have a look this evening at adding that in.

mheap commented 12 years ago

Just wondering, what's the reasoning behind not using external dependencies? Using Request it'd be as easy as doing something like:

request(fileUrl).pipe(fs.createWriteStream(output))
jackfranklin commented 12 years ago

@mheap by external dependencies I mean things you can't require() through NPM. That Request looks ace, I'll probably push a version this eve that drops wget and uses that instead. Cheers.

mheap commented 12 years ago

Request is awesome, I use it for all HTTP based stuff now.

Word of warning, though that line I shared looks awesome, it has no 404 handling etc built in. It literally takes whatever's at fileUrl and puts it in output. Will need a bit more code to make it robust.

jackfranklin commented 12 years ago

Awesome stuff, will have a play with it this evening. Will prob rewrite to use a couple of other modules I like the look of, including Commander JS too.

jackfranklin commented 12 years ago

@mheap if you see the above commit (on the develop branch), I've swapped from wget to Request, which is awesome. Need to flesh out the handling of the errors but it works.