Conversation
Cleaned up parser
| 'ah': { | ||
| alias: 'addHeader', | ||
| describe: 'Specify if a header will be added to compiled files.', | ||
| default: false, |
There was a problem hiding this comment.
I think we should add the header by default
There was a problem hiding this comment.
Idk I think it looks cheap. Like those weird video converters youtube downloaders I used as a kid and that left their watermark everywhere.
Or is there an actual benefit in having a header that im not seeing?
There was a problem hiding this comment.
No actual benefit, just some credit/advertising for us in case people see the code in other people's repositories. Also makes it clear the code is generated.
There was a problem hiding this comment.
Yeah that acutally makes sense.
But maybe reduce the it to a one liner?
I will jsut reeanble it and we can change that later if we want to....
There was a problem hiding this comment.
I am fine with changing it to be only 1 or 2 lines. I got a nice comment from @zapp-brannigan-dota about maybe adding the version number. I would say the header should at least contain the following things: 'Generated by TypescriptToLua https://github.com/Perryvw/TypescriptToLua', the date, and the version of the transpiler that was used (should be possible right?).
There was a problem hiding this comment.
Why is the date important? With it I wouldn't add the header to any files that might end up version controlled.
There was a problem hiding this comment.
Figured date would be useful for keeping track of when files changed but I guess it will also be in the file metadata/commits so maybe we can indeed leave it out.
| } else { | ||
| alias.forEach((val) => optNameMap[val] = true); | ||
| } | ||
| } |
There was a problem hiding this comment.
Very unclear what the purpose of this loop is and why it sets values to true, add some explanation.
src/CommandLineParser.ts
Outdated
| } | ||
| } | ||
|
|
||
| /** Find configFile, function form ts api seems to be broken? */ |
More cleanup Removed usage from readme
|
|
||
| NOTES: | ||
| - The tsc options might have no effect. | ||
| - Options in tsconfig.json are prioritized. |
There was a problem hiding this comment.
I think Options in tsconfig.json are prioritized. should be in here or the wiki.
There was a problem hiding this comment.
Changed it so that CLI is prioritized. This is the same behaviour now as in tsc and probably any cli tool out there. So I don't think its needed to mention that, but it wouldn't hurt either if this is mentioned somewhere in the docs.
There was a problem hiding this comment.
That's much better. It only needed mentioning because it was different from tsc.
3 participants
Cleaned up parser
Closes #68, Closes #71