lukeed / taskr

A fast, concurrency-focused task automation tool.
MIT License
2.54k stars 73 forks source link

Feature/insert #171

Closed tjbenton closed 8 years ago

tjbenton commented 8 years ago

I made some small updates to the Fly.source allowing to pass in options to globby/node-glob.

The big feature I added starts on 377 in lib/fly.js it adds the ability to modify the path before a file is output so that you can easily manage mono repos.

I added tests for it, updated the docs, as well as added an example for how it can be useful.

lukeed commented 8 years ago

Honestly, this whole insert option screams "plugin!" to me. I don't see this being useful to the 95%. @bucaran what do you think?

tjbenton commented 8 years ago

@lukeed I would make it into a plugin but I don't think there's a way to do it since the information that's passed into filters doesn't contain the path info only contains the data and the filename

tjbenton commented 8 years ago

I don't disagree with @lukeed that in most cases it's not going to be used. But I just started a monorepo in the same way that Babel did with their packages and I took their Gulpfile.js and added a few more things to in to support other output folders in nested packages and before i knew it the gulpfile was 100+ lines and doing some strange things which is the reason I abbandonded gulp in the first place. With this small addition I was able to reduce 100+ lines down into a small very manageable 20 lines.

import path from 'path'
const packages = (folder) => path.join('packages', '*', folder, '**', '*.js')

export default async function build() {
  await this
    .source(packages('src'), packages('app'))
    .target('packages', { insert: 'dist' })

  const others = [ 'scripts', 'tools', 'test' ]
  const promises = []

  for (let other of others) {
    this
      .source(packages(`${other}-src`))
      .target('packages', { insert: `${other}` })
    promises.push(other)
  }

  await Promise.all(promises)
}
lukeed commented 8 years ago

Yeah, filter doesn't expose those things that well via its params alone; but this is bound to the Fly instance, so there are ways around the limiting params:

this.pluginName = (options) => {
  console.log(this._); // access to `_`, including `globs`
  this.unwrap(files => files.map(file => console.log(file))); // full filepath
}
lukeed commented 8 years ago

@tjbenton @bucaran I came up with something that is a very minimal change, but allows for easy target() adjustments like this.

It's also highly reusable & solves some work-arounds I was implementing in other plugins.

lukeed commented 8 years ago

@tjbenton With #172 in place, this is all you have to do:


function modify(dir, name) {
  dir = dir.split(path.sep);
  dir[2] = name;
  return dir.join(path.sep);
}

exports.default = function * () {
  yield this.source('packages/*/{app,src}/**/*.js')
    .filter((data, opts) => {
      opts.file.dir = modify(opts.file.dir, 'dist');
      return data;
    })
    .target('packages');
};

I left room for that modify to be a bit more refined.

This produces:

\packages
  \pkg1
    app\scripts\1-app.js
    src\scripts\1-src.js
    dist\scripts
      1-app.js
      1-src.js
  \pkg2
    app\scripts\2-app.js
    src\scripts\2-src.js
    dist\scripts
      2-app.js
      2-src.js
ghost commented 8 years ago

Thanks for the contribution @tjbenton :)

Does @lukeed's #172 help you achieve what you need adjusting the target?

tjbenton commented 8 years ago

@bucaran Yeah it looks like that fixes my issue. In a much better way.

I can remove my code around the feature/insert and just add the rename functionality that was mentioned in #90

lukeed commented 8 years ago

Cool! I go ahead and merge that in, which will close this.

As for rename, I'm not sure a new prototype method is needed. Since the full pathObject is exposed within each filter, that allows for easy filename, extension, and directory adjustments (as above), as well as filename filtering too.

Once again, a plugin would suit that really well, just so that other people won't need to redeclare or figure out that logic on their own. But otherwise it's a simple filter and I think Fly should stop at offering the base, extendable filter () rather than embed specific filter actions.

Thanks very much for contributing! I'm sure we'll get something of yours in next time =)

If you're interested, I can post my to-do list. I havent been very good about sharing that/writing it down.