Fixed #241 add new option newLineExemptFileExts

[lfurzewaddock]

I've added 1 new option: newLineExemptFileExts (optional), which may be passed in addition to the newLine option which specifies which file extensions should ignore the newLine option. It uses a case insensitive RegExp to match the file extensions, with/without the leading full stop/period.

I've added 2 new passing tests, but had some trouble with the test suite, as when a test failed, all the remaining tests would fail, but only after timing out. I'm not sure if this is the expected behaviour, or if there is anything I should/can do?

[coveralls]

Coverage increased (+0.08%) to 98.225% when pulling d27377eaedc09fd2fe4c3b01e3eb842801555a20 on lfurzewaddock:pr/newLineExemptFileExts into 53efa13d6c276be46fabbafdff08f0882c98a9a2 on jonkemp:master.

[jonkemp]

I have a proposed fix for #241 here. Let me know if this would meet the needs. Css files don't really need the newline character at all. But it's been there a long time and I don't want to break anybody's code.

[lfurzewaddock]

I discovered issue #241 myself in a legacy project I was working on, when after several hours I had a Homer moment when I realised semicolons I had set to fix the JS bundle were being inserted into the CSS bundle too.

Originally, I coded a simple fix, but then realised this plugin is used with a wide range of stacks, not just the usual HTML/CSS/JS, which is why this PR, is an additive non-breaking change, that allows more flexibility to accommodate other use cases/stacks I'm not aware of.

The ultimate flexible solution, although I'm not sure is required, would be a hash table option to allow 'newLine' character(s)/codes to be set per file extension.

[jonkemp]

That's the thing. I can't find any instances of anyone using a newline character to combine css files. I see it more as fixing a bug to allow the feature to be used correctly. I would rather take that approach before coding a feature that is not needed.

[lfurzewaddock]

I've just had a quick look again at PR #249. This condition uses an OR operator. I assume this should be an AND operator?

if (options.hasOwnProperty('newLine') || type === 'js') {
         gulpConcatOptions.newLine = options.newLine;
     }

I agree a fix should be kept simple, but PR #249, (assumption above), will only apply newLine to js files exclusively, which I think is too restrictive. If you're 100% sure the newLine option will never be used with CSS files you could modify your PR to exclude only CSS files, but this again is restrictive and limits options for users outside of the traditional HTML/CSS/JS stack. However, another simple alternative, which would have fixed my original problem, is to strip/filter the semicolon character from a newLine option string, before passing it to gulp-concat for CSS files.

The problem is, I cannot be sure how gulp-useref is used in the wild and for which file types. This is why PR #248 is flexible enough to allow users to continue without making any changes but gives them the opportunity to apply or not apply the newline option to files with any extension. However, as I said, the ultimate solution to achieve fine-grained control, would be a hash table containing file extension and newLine option string pairs.

[jonkemp]

However, another simple alternative, which would have fixed my original problem, is to strip/filter the semicolon character from a newLine option string, before passing it to gulp-concat for CSS files.

Ok, I agree with this. I've updated #249.

[lfurzewaddock]

OK @jonkemp, I made a small suggestion on PR #249

[jonkemp]

https://github.com/jonkemp/gulp-useref/pull/249 is merged.