Windows BOM

[jimisaacs]

Why was this considered ok? https://github.com/gruntjs/grunt/blob/master/lib/grunt/file.js#L228-L231

When writing a Windows Store app, and ran through WACK, it is required that files contain a BOM, and fails certification otherwise.

If iconv was exported I may have found a better short term solution fix, but without it I had to write my own tasks for many things.

Also separate topic, how do people feel about the idea of dropping every 3rd party dependency in one namespace such as grunt.modules or grunt.internal.

[shama]

See #338. It fixed way more problems than it caused. To date you're the first use case that needs a BOM. :)

It seems fairly simple to just add a BOM where it's needed though. Why not just write a task that adds BOMs where needed to pass certification? Unless I'm completely missing something.

[jimisaacs]

That's what I did, which is on every text file. There's a problem with this because using grunt.file.read I have no way to actually check whether a file has a BOM or not because it is being stripped, unless I send an encoding of null, which I had to view the source to find out.

So using iconv, I just decode the buffer just like in grunt.file.read, and check myself. Which brought me to the second point of exposing all 3rd party plugins used by grunt. How about an optout option? But really the ideal here, would be to use the process option on grunt.file.copy to check whether to add a BOM, or abort the write. Which I could do by sending an encoding of null, and having iconv there to do the same check as in grunt.file.read. So yes, I can install iconv separately, but would like it exposed since it's already there.

Does this all make sense?

[jimisaacs]

I know development on windows doesn't mean you are targeting windows, but in this case, I guess I am the first doing that with grunt?

[shama]

I don't think Grunt should expose its own dependencies because then users would have to wait for us to publish releases of Grunt to update their libs. npm will sort of do this automatically though. If you install a module with the same version before grunt, it will only install it in one place. But installing a tiny module in two locations isn't a big deal anyways.

There are many people using Grunt on Windows, even some of the core contributors are on Windows. I meant you're the first person to ask for the BOM to not be automatically removed. As in most cases programs shouldn't really rely on it being there and don't. So it is a rather useless mark and safe to remove. The Windows Store requiring it is an edge case, imo.

But I am +1 for an option to opt out of it being automatically removed.

[cowboy]

I can see there being an option similar to grunt.file.defaultencoding that one can set to preserve BOM. Maybe grunt.file.preserveBOM.

[jimisaacs]

@cowboy This is not urgent, as I do have a working solution right now, but I am +1 for an option like that.

[Lucasus]

I've similar problem as @jimisaacs , I'm writing Win8 Store app too, and an option to preserve BOM would be very helpful.

[cowboy]

@jimisaacs @Lucasus I know nothing about BOM, so I need your input. Is the solution as simple as don't strip BOM from files when reading their contents?

Because if so, I'll add the grunt.file.preserveBOM boolean option and change this code from:

if (contents.charCodeAt(0) === 0xFEFF) {
  contents = contents.substring(1);
}

to:

if (!file.preserveBOM && contents.charCodeAt(0) === 0xFEFF) {
  contents = contents.substring(1);
}

Feel free to test this by temporarily removing those 3 lines from your local Grunt lib\grunt\file.js and bypassing your custom BOM-adding tasks. And if you want credit for the fix, make these changes yourself and submit a PR.

Either way, if you respond today, this stands a very good chance of making it into the upcoming 0.4.2 release!

[jimisaacs]

@cowboy +1 LGMT, as long as we can pass this option down easy enough.
Would this work?

grunt.file.copy(srcpath, destpath, {preserveBOM: true});

I'm thinking not because of this https://github.com/gruntjs/grunt/blob/master/lib/grunt/file.js#L305
Does that mean this would need to be done?

grunt.file.copy(srcpath, destpath, {preserveBOM: true, process: function (contents) { return contents; }});

This may be a separate issue, but at the same time, this issue is effected most by the copy task because that's where the BOM is stripped unexpectedly. How about:

// does it really matter that `options.process` is passed on?
if (!process) options.encoding = null;

[cowboy]

What I'm saying is that you'd set this at the top of your Gruntfile:

grunt.file.preserveBOM = true;

And Grunt would preserve BOM in all file read operations, automatically. It would work like the existing grunt.file.defaultencoding method.

Will that work for you?

[nitriques]

+1 for it being customizable.
+1 if it's on by default. I like to be consistent with those.

[jimisaacs]

@cowboy Ah ha, totally read it wrong sorry.
So yeah, still +1 LGMT, and that's totally easy enough to define. Thanks.

[cowboy]

@nitriques It won't be on by default, you'll have to enable it. But once enabled, it will stay on for all file read / copy operations done through grunt.file methods.

@jimisaacs have you tested it? Do you want to submit a PR? 😀

[jrschumacher]

+1 for being customizable (off by default)

[nitriques]

@cowboy Great. would this apply to any other apps that uses grunt (like Yeoman). (I think they use grunt.file method a lot...)

[jimisaacs]

@cowboy coming up