TypeStrong / grunt-ts

A grunt task to manage your complete typescript development to production workflow
https://www.npmjs.com/package/grunt-ts
MIT License
330 stars 121 forks source link

newLine switch is removed on Windows platforms #313

Closed Maks3w closed 8 years ago

Maks3w commented 8 years ago

This optimization was buggy and remove the switch on any platform

I've tried to set newLine in tsconfig and as grunt-ts setting and the flag was removed always.

As workaround I've to enable passThrough (why is not set by default?)

        tsconfig: {
          passThrough: true
        }
nycdotnet commented 8 years ago

Hi - thanks for submitting this. When you say it was buggy, can you elaborate?

Maks3w commented 8 years ago

I was trying to use LF in a Windows platform with Grunt linefeed set to '\n'. Result is newLine switch is not set.

It's clear that optimization does not work. Also I don't understand why grunt-ts should be responsible of "remove" that switch.

nycdotnet commented 8 years ago

Understood.

We have a lot of stuff in the codebase for legacy compatibility, and grunt-ts does need to know the intended newline style to handle things like transforms.

I hear you saying that you've set grunt.util.linefeed to '\n', but it's not passing through on Windows. I'd like to investigate that issue before just removing this call because it did solve a problem for a user when we first implemented it.

Thanks for the PR - I'll look into it ASAP.

Maks3w commented 8 years ago

I've set grunt.util.linefeed in Gruntfile.js and newLine in tsconfig.json

nycdotnet commented 8 years ago

Hi @Maks3w,

Thanks for bringing this to my attention. I believe I may have fixed the issue. The trouble was that we were using grunt.util.linefeed as the default linefeed for the OS, but of course it's reasonable that someone might change that for use with other Grunt tasks. I've changed the function to query the OS directly as this is what TSC does. I've also added tests.

Can you please check out grunt-ts@5.3.0-beta.1 and let me know if it resolves your issue? If not, please open an issue and let's discuss further. Thanks again for your contribution! I've added a credit for you to the Changelog file.

Maks3w commented 8 years ago

I still don't understand why grunt-ts should check if the parameter is redundant. Also I guess the only value redundant is CRLF no matter in which platform is executed.

nycdotnet commented 8 years ago

I'm operating under the assumption that on Mac/Linux, the default for TS is to use LF and on Windows it's CRLF. Is that not your observation? That's how it was when we implemented this.