Have nonull: true cause the task to warn and abort.

[jacksonrayhamilton]

Previously, specifying nonull: true would only cause the task to log a warning about a file being missing. This change modifies the task to issue a "real" warning and abort the task chain.

It is rather nasty when concatenation skips over a file because you forgot to include it in your project. Say a developer is using bower link to develop some bower packages and is concatenating all his libraries into a vendors.js file. On a fresh clone he might forget to run bower link package-name, and then be confused by the fact that vendors.js is generated but is missing a package.

The current behavior of nonull: true begins to solve this problem, unless you have a long list of tasks and the warning gets buried in your console history. Having a warning at the tip of your console history immediately alerts you of a potential issue. Also, considering you specified the nonull: true option, you have acknowledged that some paths have the potential to not match and would really, really like to know if they don't.

I imagine it may have been difficult to test this functionality before (if anyone had ever desired it) because the current set of tests analyze the output of the main Gruntfile's initConfig concat. I was able to test warnings and task-abortion by analyzing the stdout of child_process#exec.

Please consider this pull request. I believe that fail-fast functionality is much more efficient for development.

[jacksonrayhamilton]

This fix just saved me in my own project. Please give this request some attention. Thanks.

[dbo]

+1 for this change, saves my life. Really would like to see this upstream.

[XhmikosR]

@jacksonrayhamilton: you have many unrelated changes in this branch. You should rebase and clean it up. Then, someone might actually be able to have a look.

[nchase]

@jacksonrayhamilton can you get back to this? 👍

[jacksonrayhamilton]

Cleaned it up.

[XhmikosR]

The branch still has unrelated changes.
On Feb 19, 2015 7:24 PM, "Jackson Ray Hamilton" notifications@github.com
wrote:

Cleaned it up.

—
Reply to this email directly or view it on GitHub
#106 (comment)
.

[jacksonrayhamilton]

@XhmikosR, I don't understand, what is unrelated?

[XhmikosR]

You should just have the fail warm change and not all other stuff. If you
want to add a test you should do it in a less intrusive way.
On Feb 19, 2015 9:31 PM, "Jackson Ray Hamilton" notifications@github.com
wrote:

@XhmikosR https://github.com/XhmikosR, I don't understand, what is
unrelated?

—
Reply to this email directly or view it on GitHub
#106 (comment)
.

[jacksonrayhamilton]

There was a limitation in the way the tests were set up. As I explained,

. . . the current set of tests analyze the output of the main Gruntfile's initConfig concat. I was able to test warnings and task-abortion by analyzing the stdout of child_process#exec.

I wanted to actually know that the task failed and aborted. But normally a failing task would cause the whole test suite to fail, as the very intent of this pull request is to make a chain of grunt tasks fail! Thus I had to sandbox the failure inside a child process.

[sindresorhus]

👍 Can you fix the merge conflict?

[dbo]

@jacksonrayhamilton Any chance you proceed on this anytime soon? Thanks in advance! 👍

[tkambler]

I believe this PR should supersede this one. It solves the same problem in a strictly backwards-compatible way with only six lines of code. Any chance we can get some movement on this?

[jacksonrayhamilton]

Hey guys, sorry that this issue got abandoned. I totally forgot about it since I haven't been using this package as much as I used to.

@tkambler, I do like that your solution is backwards-compatible. I don't think it's fair to say that it's a lot simpler, since it doesn't include tests or documentation. But if you added those things, I would definitely be in support of your change over this one.

[jacksonrayhamilton]

It looks like this has been a visible issue for some time. There was discussion about adding a nonullFail option, which doesn't seem to be resolved due to a paradox @cowboy brought up: gruntjs/grunt#1105. If that issue were ever implemented, it would probably supercede a nonull: 'error' option.

There was also a discussion about whether tasks should fail or warn: gruntjs/grunt#1163 It also wasn't resolved, but it took place several years ago, so unless the grunt authors want to chime in, I think it's about time we took action.

It was said that the ideal behavior would be, "A task should fail on anything that prevents it from accomplishing its primary task." I suppose it's true that "a concatenated file could still be made even if some files were missing," but it doesn't seem true that "the concatenation of a set of files can include only a subset of the files." In the latter case, which seems more pressing to me, this PR is probably correct, even though it might be a breaking change.

[jacksonrayhamilton]

On Node.js 0.10 on Windows, due to this issue, concatenated files were still being generated even when the task was instructed to fail. (This is why that test was failing on Appveyor.)

Added an unsightly workaround for it. Certainly open to other ideas for that fix. Might be something that should handled in the main grunt package too, if faithful 0.10 Windows support is desirable. Given that support for it will be cut in a few months, might help our sanity to turn a blind eye to that environment, at least with respect to this bug.

[jacksonrayhamilton]

Bump. Anyone still interested in this?

[dbo]

sure