twolfson / grunt-curl

Download files from the internet via grunt.
The Unlicense
73 stars 28 forks source link

Shouldn't have to specify destination file name #8

Closed jasonkarns closed 11 years ago

jasonkarns commented 11 years ago

I would expect the following to work:

curl: {
  jquery: {
    src: 'http://code.jquery.com/jquery-1.10.1.js',
    dest: 'vendor/js/'
  },
  underscore: {
    src: 'http://underscorejs.org/underscore.js',
    dest: 'vendor/js/'
  }
}

I would expect grunt curl:jquery to result in vendor/js/jquery-1.10.1.js. According to the documentation, this should already be possible, but I presume it only works when src has more than one file. I even tried changing src to a single-element array to no avail.

Though, while we're on the subject, since this is a multiTask, shouldn't the destination directory be configurable at the top level?

curl: {
  options: { dest: 'vendor/js/' },

  jquery: { src: 'http://code.jquery.com/jquery-1.10.1.js' },
  underscore: { src: 'http://underscorejs.org/underscore.js'  }
}
twolfson commented 11 years ago

The functionality you are describing is that of curl-dir which can be found in the README as well.

I am not too familiar with the top-level configuration you are referencing, can you point me to some code/docs?

jasonkarns commented 11 years ago

The functionality you are describing is that of curl-dir which can be found in the README as well.

Exactly. I don't see why curl and curl-dir shouldn't be merged. Most grunt tasks are designed such that providing a single file argument, or an array of files, or even glob patterns all behave consistently. For instance, (example paraphrased from http://gruntjs.com/configuring-tasks)

curl: {
  foo: {
    files: [
      {src: 'src/a.js', dest: 'dest/' },
      {src: 'src/1.js', dest: 'dest/1_prime.js' }
    ]
  },

  bar: {
    files: [
      {src: ['src/1.js', 'src/2.js'], dest: 'dest/' },
      {src: 'src/a.js', dest: 'dest/'}
    ]
  }
}

If this format were used for grunt-curl, I would expect grunt curl:foo to create:

dest/
  1_prime.js
  a.js

and grunt curl:bar to create:

dest/
  1.js
  2.js
  a.js

Maybe I'll take a crack at a pull request later this week.

jasonkarns commented 11 years ago

Regarding the base options shared across multiple targets:

from the documentation: http://gruntjs.com/configuring-tasks#options a grunt-contrib-jshint example: https://github.com/gruntjs/grunt-contrib-jshint#specifying-jshint-options-and-globals

twolfson commented 11 years ago

To your first reply, regarding merging curl and curl-dir

I broke up the two tasks to make them easily digestible and approachable. We, engineers, have a problem with clumping items for a more elegant API but dirtier implementation. This can also lead to odd edge cases and frustrates everyone. This is the exact reason I split them up.

I will give you an example to demonstrate my point

{src: 'mysite/a.js', dest: 'public/a.js'} // Write out a.js to public/a.js
{src: 'mysite/a.js', dest: 'public/'} // Write out a.js to public/
{src: 'mysite/a.js', dest: 'public'} // Write out a.js as public
{src: ['mysite/a.js', 'mysite/b.js'], dest: 'public/'} // Write out a.js, b.js to public/
{src: ['mysite/a.js', 'mysite/b.js'], dest: 'public'} // Error: public/ is missing a trailing slash.

For the last item, there are many potential arguments on how to handle it (and I guarantee you there is someone that argues for each):

As a result of this massive rabbit hole, I decide to step around it completely and break it into two tasks. This leaves the decision at the feet of the developer who already has his own opinion (e.g. yourself).

To your second reply, regarding options

options are additional parameters you can provide to affect the way a task is run. However, fundamental items like src and dest cannot be clumped via options.

In addition, if that functionality has been introduced in grunt@0.4, I remain firm that grunt@0.3 did some nice things that were lost along the way. As a result, I would lose that functionality in forfeit to grunt@0.3 support.

Thank you for voicing your opinion and feel free to respond.

jasonkarns commented 11 years ago

Regarding the options, yes, I complete misconstrued the ability to put file-options into the generic options hash.