Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ability to not overwrite config of a task #36

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

kfiku
Copy link

@kfiku kfiku commented Mar 3, 2014

I Propose to add the ability to not overwrite config of a task.
There are a tasks like grunticon witch need original config (with all files) to work right - so grunt-newer will run full task if any file will change.
To ignore changing config (line 120) need to add in never task options changeConfig: false.
You can specify changeConfig to all task, or to single taks:

newer: {
    options: {
        cache: 'app/cache/grunt',
        changeConfig: false
    }
}

or

newer: {
    options: {
        cache: 'app/cache/grunt',
        grunticon: {
            changeConfig: false
        }
    }
}

I Propose to add the ability to not overwrite config of a task.
There are a tasks like `grunticon` witch need original config (with all files) to work right - so `grunt-newer` will run full task if any file will change.
To ignore changing config (line 120) need to add in never task options `changeConfig: false`.
You can specify `changeConfig` to all task, or to single taks:
```
newer: {
    options: {
        cache: 'app/cache/grunt',
        changeConfig: false
    }
}
```
or
```
newer: {
    options: {
        cache: 'app/cache/grunt',
        grunticon: {
            changeConfig: false
        }
    }
}
```
@tschaub
Copy link
Owner

tschaub commented Mar 3, 2014

The way this is implemented, it looks like it would have the same effect as not prefixing your grunticon task with newer.

The point of overwriting the config is so a task works with a limited set of files (only those that are newer). If you don't modify the config, the newer task will have no effect.

Can you give an example of your grunticon config and describe how you expect things to behave with regard to modifying files?

@kfiku
Copy link
Author

kfiku commented Mar 4, 2014

I known what is newer :) it's grate think, but in grunticon situation I need to run grunticon only if any source file change, but full task, with all files.

grunticon is task that manage svg/png files for all devices and generating png/svs/css for it. So you can't go only with, for example, 3 of 10 images, becouse you end up with css classes for 3 images.

In grunticon like in other css sprite generators images for it is not changing very often, but you need this task in build process. It's also take a while to process (in my situation is 43sec.).
So if we can make it faster - check if any file is chcange - it will be very cool.

Example grunticon task

grunticon: {
    options: {
        pngfolder: '',
        template: '<%= conf.app %>/images/sprite/template.hbs'
    },
    icons: {
        files: [{
            expand: true,
            cwd: '<%= conf.app %>/images/sprite',
            src: ['*.svg', '*.png'],
            ext: '.png',
            dest: '<%= conf.app %>/images/generated'
        }],
    }
};

@mattkime
Copy link

+1 @kfiku

I'm interested in this functionality as well. I'm taking a look at your patch to see if it solves my problem as well.

thanks!

@kfiku
Copy link
Author

kfiku commented Apr 23, 2014

@mattkime we are blocked by @tschaub. I think @tschaub don't see a lot of use cases where this feature can be grate. Also grate can be somethink like hybrid with grunt-never and https://github.com/rse/grunt-newer-explicit

@tschaub
Copy link
Owner

tschaub commented Apr 28, 2014

If newer is used in front of a task with a many:1 source:dest file mapping, the task will be run with all source files if any one of them has changed. This sounds like the behavior you want with the grunticon task.

However, with the above configuration, because expand is true in the files config, this looks like a 1:1 src:dest mapping to Grunt (for each source file, one dest file will be generated). It looks like the grunticon task is a special case that doesn't generate 1 dest file for each source file or 1 dest file for many source files. Instead it generates a few dest files for many source files.

Previously (@v0.5.4) there was an any-newer task that would handle cases like this (including all source files if any one of them was newer than a dest file). I'd be curious to hear if using the any-newer task (in the 0.5.4 release) works for you.

@kfiku
Copy link
Author

kfiku commented Apr 30, 2014

@tschaub I see you still don't understand how grunticon works and how my commit allow newer works with it.
Grunticon must be run with all files because it generates ont only one file from many and not many to many. You specify folder where you have svg|png|jpg... files that you want to mage sprite from, next it generate: 3 css files (3 options), png fallbacks for older browsers.

So again:

  • you need all files (other way you get not all file in sprite)
  • you don't have many to many
  • you don't have one to meny
  • you need to check if any file was change, if so run it with fill config, other way don't run.

You say:
The way this is implemented, it looks like it would have the same effect as not prefixing your grunticon task with newer
But no, svg sprite source files don't change very often, but we need grunticon in build taks and it can be very long (10-50s.). This update will speed think up

@mattkime
Copy link

I believe the options.override feature solves this general problem. move to close this pr but i wanted to give @kfiku a heads up

@@ -40,6 +40,26 @@ function createTask(grunt) {
var options = this.options({
cache: path.join(__dirname, '..', '.cache')
});

if(options[taskName]) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this idea

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants