Return downloaders :sparkles:

5 files changed, 174 insertions, 8 deletions

diff --git a/package.json b/package.json
index 92f11ea..45dbf62 100644
--- a/package.json
+++ b/package.json
@@ -20,6 +20,7 @@
   "dependencies": {
     "cheerio": "^1.0.0-rc.1",
     "fifo-js": "^2.1.0",
+    "fs-extra": "^3.0.1",
     "ncp": "^2.0.0",
     "node-fetch": "^1.7.0",
     "node-natural-sort": "^0.8.6",
diff --git a/src/downloaders.js b/src/downloaders.js
index 8fa830c..1de6730 100644
--- a/src/downloaders.js
+++ b/src/downloaders.js
@@ -1,6 +1,7 @@
 'use strict'
 
 const fs = require('fs')
+const fse = require('fs-extra')
 const fetch = require('node-fetch')
 const promisifyProcess = require('./promisify-process')
 const tempy = require('tempy')
@@ -11,6 +12,7 @@ const { spawn } = require('child_process')
 const { promisify } = require('util')
 
 const writeFile = promisify(fs.writeFile)
+const copyFile = fse.copy
 
 function makeHTTPDownloader() {
   return function(arg) {
@@ -42,6 +44,32 @@ function makeYouTubeDownloader() {
 }
 
 function makeLocalDownloader() {
+  // Usually we'd just return the given argument in a local
+  // downloader, which is efficient, since there's no need to
+  // copy a file from one place on the hard drive to another.
+  // But reading from a separate drive (e.g. a USB stick or a
+  // CD) can take a lot longer than reading directly from the
+  // computer's own drive, so this downloader copies the file
+  // to a temporary file on the computer's drive.
+  // Ideally, we'd be able to check whether a file is on the
+  // computer's main drive mount or not before going through
+  // the steps to copy, but I'm not sure if there's a way to
+  // do that (and it's even less likely there'd be a cross-
+  // platform way).
+
+  return function(arg) {
+    const dir = tempy.directory()
+    // TODO: Is it necessary to sanitize here?
+    // haha, the answer to "should I sanitize" is probably always
+    // YES..
+    const base = path.basename(arg, path.extname(arg))
+    const file = dir + '/' + sanitize(base) + '.mp3'
+    return copyFile(arg, file)
+      .then(() => file)
+  }
+}
+
+function makeLocalEchoDownloader() {
   return function(arg) {
     // Since we're grabbing the file from the local file system, there's no
     // need to download or copy it!
@@ -72,7 +100,7 @@ module.exports = {
   makeLocalDownloader,
   makePowerfulDownloader,
 
-  getDownloader: downloaderType => {
+  getDownloaderFor: downloaderType => {
     if (downloaderType === 'http') {
       return makeHTTPDownloader()
     } else if (downloaderType === 'youtube') {
@@ -82,5 +110,17 @@ module.exports = {
     } else {
       return null
     }
+  },
+
+  getDownloaderFor(arg) {
+    if (arg.startsWith('http://') || arg.startsWith('https://')) {
+      if (arg.includes('youtube.com')) {
+        return makeYouTubeDownloader()
+      } else {
+        return makeHTTPDownloader()
+      }
+    } else {
+      return makeLocalDownloader()
+    }
   }
 }
diff --git a/src/loop-play.js b/src/loop-play.js
index 10b1d5f..aca518a 100644
--- a/src/loop-play.js
+++ b/src/loop-play.js
@@ -3,24 +3,111 @@
 const { spawn } = require('child_process')
 const FIFO = require('fifo-js')
 const EventEmitter = require('events')
+const { getDownloaderFor } = require('./downloaders')
+
+class DownloadController extends EventEmitter {
+  waitForDownload() {
+    // Returns a promise that resolves when a download is
+    // completed.  Note that this isn't necessarily the download
+    // that was initiated immediately before a call to
+    // waitForDownload (if any), since that download may have
+    // been canceled (see cancel).  You can also listen for the
+    // 'downloaded' event instead.
+
+    return new Promise(resolve => {
+      this.once('downloaded', file => resolve(file))
+    })
+  }
+
+  async download(downloader, arg) {
+    // Downloads a file.  This doesn't return anything; use
+    // waitForDownload to get the result of this.
+    // (The reasoning is that it's possible for a download to
+    // be canceled and replaced with a new download (see cancel)
+    // which would void the result of the old download.)
+
+    let canceled = false
+    this.once('skipped', () => {
+      canceled = true
+    })
+
+    const file = await downloader(arg)
+
+    if (!canceled) {
+      this.emit('downloaded', file)
+    }
+  }
+
+  cancel() {
+    // Cancels the current download.  This doesn't cancel any
+    // waitForDownload promises, though -- you'll need to start
+    // a new download to resolve those.
+
+    this.emit('skipped')
+  }
+}
 
 class PlayController {
-  constructor(picker) {
+  constructor(picker, downloadController) {
     this.currentTrack = null
     this.playArgs = []
     this.process = null
     this.picker = picker
+    this.downloadController = downloadController
   }
 
   async loopPlay() {
+    let next
+
+    let downloadNext = () => {
+      if (this.startNextDownload() !== null) {
+        return this.downloadController.waitForDownload().then(_next => {
+          next = _next
+        })
+      } else {
+        next = null
+        return Promise.resolve()
+      }
+    }
+
+    await downloadNext()
+
+    while (next) {
+      await Promise.all([
+        this.playFile(next),
+        downloadNext()
+      ])
+    }
+  }
+
+  startNextDownload() {
+    // TODO: Is there a method for this?
+    // TODO: Handle/test null return from picker.
+    const arg = this.picker()[1]
+
+    if (arg === null) {
+      return null
+    } else {
+      const downloader = getDownloaderFor(arg)
+      this.downloadController.download(downloader, arg)
+    }
+  }
+
+  async old_loopPlay() {
     // Playing music in a loop isn't particularly complicated; essentially, we
     // just want to keep picking and playing tracks until none is picked.
 
     let nextTrack = await this.picker()
 
+    await this.downloadManager.download(getDownloaderFor(nextTrack), nextTrack)
+
+    let downloadNext
+
     while (nextTrack) {
       this.currentTrack = nextTrack
 
+      this.downloadManager.download(getDownloaderFor(nextTrack), nextTrack)
+
       await this.playFile(nextTrack[1])
 
       nextTrack = await this.picker()
@@ -35,7 +122,7 @@ class PlayController {
       file
     ])
 
-    this.process.stderr.on('data', data => {
+    const handleData = data => {
       const match = data.toString().match(
         /(..):(..):(..) \/ (..):(..):(..) \(([0-9]+)%\)/
       )
@@ -70,6 +157,12 @@ class PlayController {
           `\x1b[K~ (${percentStr}%) ${curStr} / ${lenStr}\r`
         )
       }
+    }
+
+    this.process.stderr.on('data', handleData)
+
+    this.process.once('exit', () => {
+      this.process.stderr.removeListener('data', handleData)
     })
 
     return new Promise(resolve => {
@@ -136,7 +229,8 @@ module.exports = function loopPlay(picker, playArgs = []) {
   // function is null (or similar). Optionally takes a second argument
   // used as arguments to the `play` process (before the file name).
 
-  const playController = new PlayController(picker)
+  const downloadController = new DownloadController()
+  const playController = new PlayController(picker, downloadController)
   playController.playArgs = playArgs
 
   const promise = playController.loopPlay()
diff --git a/todo.txt b/todo.txt
index e6c4c76..ba57347 100644
--- a/todo.txt
+++ b/todo.txt
@@ -166,3 +166,11 @@ TODO: Figure out a way to make the same mpv process be reused, so that options
 
 TODO: Figure out how to stream audio data directly, or at least at a lower
       level (and stupider, as in "man git" stupid).
+
+TODO: Validate paths in getDownloaderFor, maybe?
+
+TODO: Figure out the horrible, evil cause of the max listeners warning
+      I'm getting lately.. current repro: play a bunch of files locally.
+      Skipping tracks still leads to issue. Wait for them to start playing
+      before skipping, though.
+
diff --git a/yarn.lock b/yarn.lock
index 86bf07a..6e61ed0 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -1,5 +1,7 @@
 # THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
 # yarn lockfile v1
+
+
 "@types/node@^6.0.46":
   version "6.0.73"
   resolved "https://registry.yarnpkg.com/@types/node/-/node-6.0.73.tgz#85dc4bb6f125377c75ddd2519a1eeb63f0a4ed70"
@@ -44,14 +46,14 @@ css-what@2.1:
   version "2.1.0"
   resolved "https://registry.yarnpkg.com/css-what/-/css-what-2.1.0.tgz#9467d032c38cfaefb9f2d79501253062f87fa1bd"
 
-dom-serializer@~0.1.0, dom-serializer@0:
+dom-serializer@0, dom-serializer@~0.1.0:
   version "0.1.0"
   resolved "https://registry.yarnpkg.com/dom-serializer/-/dom-serializer-0.1.0.tgz#073c697546ce0780ce23be4a28e293e40bc30c82"
   dependencies:
     domelementtype "~1.1.1"
     entities "~1.1.1"
 
-domelementtype@^1.3.0, domelementtype@1:
+domelementtype@1, domelementtype@^1.3.0:
   version "1.3.0"
   resolved "https://registry.yarnpkg.com/domelementtype/-/domelementtype-1.3.0.tgz#b17aed82e8ab59e52dd9c19b1756e0fc187204c2"
 
@@ -65,7 +67,7 @@ domhandler@^2.3.0:
   dependencies:
     domelementtype "1"
 
-domutils@^1.5.1, domutils@1.5.1:
+domutils@1.5.1, domutils@^1.5.1:
   version "1.5.1"
   resolved "https://registry.yarnpkg.com/domutils/-/domutils-1.5.1.tgz#dcd8488a26f563d61079e48c9f7b7e32373682cf"
   dependencies:
@@ -92,6 +94,18 @@ fifo-js:
   dependencies:
     es6-error "^3.0.1"
 
+fs-extra@^3.0.1:
+  version "3.0.1"
+  resolved "https://registry.yarnpkg.com/fs-extra/-/fs-extra-3.0.1.tgz#3794f378c58b342ea7dbbb23095109c4b3b62291"
+  dependencies:
+    graceful-fs "^4.1.2"
+    jsonfile "^3.0.0"
+    universalify "^0.1.0"
+
+graceful-fs@^4.1.2, graceful-fs@^4.1.6:
+  version "4.1.11"
+  resolved "https://registry.yarnpkg.com/graceful-fs/-/graceful-fs-4.1.11.tgz#0e8bdfe4d1ddb8854d64e04ea7c00e2a026e5658"
+
 htmlparser2@^3.9.1:
   version "3.9.2"
   resolved "https://registry.yarnpkg.com/htmlparser2/-/htmlparser2-3.9.2.tgz#1bdf87acca0f3f9e53fa4fcceb0f4b4cbb00b338"
@@ -119,6 +133,12 @@ isarray@~1.0.0:
   version "1.0.0"
   resolved "https://registry.yarnpkg.com/isarray/-/isarray-1.0.0.tgz#bb935d48582cba168c06834957a54a3e07124f11"
 
+jsonfile@^3.0.0:
+  version "3.0.1"
+  resolved "https://registry.yarnpkg.com/jsonfile/-/jsonfile-3.0.1.tgz#a5ecc6f65f53f662c4415c7675a0331d0992ec66"
+  optionalDependencies:
+    graceful-fs "^4.1.6"
+
 lodash@^4.15.0:
   version "4.17.4"
   resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.4.tgz#78203a4d1c328ae1d86dca6460e369b57f4055ae"
@@ -214,6 +234,10 @@ unique-string@^1.0.0:
   dependencies:
     crypto-random-string "^1.0.0"
 
+universalify@^0.1.0:
+  version "0.1.0"
+  resolved "https://registry.yarnpkg.com/universalify/-/universalify-0.1.0.tgz#9eb1c4651debcc670cc94f1a75762332bb967778"
+
 utf8-byte-length@^1.0.1:
   version "1.0.4"
   resolved "https://registry.yarnpkg.com/utf8-byte-length/-/utf8-byte-length-1.0.4.tgz#f45f150c4c66eee968186505ab93fcbb8ad6bf61"
@@ -227,4 +251,3 @@ xmldoc:
   resolved "https://registry.yarnpkg.com/xmldoc/-/xmldoc-1.1.0.tgz#25c92f08f263f344dac8d0b32370a701ee9d0e93"
   dependencies:
     sax "^1.2.1"
-