search

mathiasbynens / luamin

A Lua minifier written in JavaScript
https://mths.be/luamin
MIT License
218 stars 55 forks source link

Option to preserve new lines? #56

Open martin-braun opened 6 years ago

martin-braun commented 6 years ago

Lua is easier to debug if the new lines could be preserved. The LuaVM returns the line number in which line the error happened, so please give us an option to preserve new lines when minifying the code using luamin.

hsandt commented 5 years ago

I'm currently working on an option to add '\n' as a separator between statements instead of ';'. It seems AST only cares about statements and expressions and lost newline information, so this won't get you the exact line number (because double newlines are stripped and if you passed a ';' originally, it will be converted to '\n' whatsoever) but it may make the code readable enough so you can identify the problematic line in its non-minified form.

Ideally we should generate a minification mapping (as in e.g. minified JS when debugging from browser). The best mapping can map columns, but combined with my option, a line mapping would be enough.

martin-braun commented 5 years ago

I'm currently working on an option to add '\n' as a separator between statements instead of ';'. It seems AST only cares about statements and expressions and lost newline information, so this won't get you the exact line number (because double newlines are stripped and if you passed a ';' originally, it will be converted to '\n' whatsoever) but it may make the code readable enough so you can identify the problematic line in its non-minified form.

Ideally we should generate a minification mapping (as in e.g. minified JS when debugging from browser). The best mapping can map columns, but combined with my option, a line mapping would be enough.

Yes, that's correct and great this suggestion is recognized, eventually.

However, in the Lua environment mapping is no automated process, like in the JS environment on browsers. So it would make life way easier, when there simply was an option to preserve new lines when using \n as separator, so even lines where comments were are preserved.

Of course, it won't look minified when you check the minified file, but one character per line is a fine trade-off to preserve easy error tracking.

Currently, I'm not minifying my scripts just because of this reason only.

hsandt commented 5 years ago

OK, it seems comments are lost in the first token parsing so I don't think I'll be able to, say, keep blank lines instead of line comments for a perfect mapping.

I have a WIP: https://github.com/hsandt/luamin/tree/feature/newline-separator which comes from my very experimental https://github.com/hsandt/luamin/tree/feature/preserve-options (just check the last commit of each branch).

The preserve-options adds a number of different options including newline preservation, but uses a hardcoded associative array for parameters so you must change them manually in the script when your preferences change... It was enough for local usage, but I wanted to make a proper option, which I am doing in the newline-separator branch.

(I did this for a project on PICO-8 as it really requires small code, but without the newlines it's really hard to debug.)

But the option parsing is still messy, you must not put -n after a filename or it will get interpreted as a filename... The current custom parser is not good enough to take multiple options at arbitrary positions. I started playing with nopt but it doesn't take multiple arguments after an option as when passing files or AST. However, I can simulate this behavior by making them positional arguments instead of option arguments.

This will take more time than I expected, but at least you can check my branches for your local usage (you can even hardcode the preserveNewLines option as in the second branch).

hsandt commented 5 years ago

I did it, but it relies on a previous PR adding nopt, which has not been accepted yet. I'm not sure if it's good to send a PR before a dependee PR has been accepted, but at least I give you the branch here: https://github.com/hsandt/luamin/tree/feature/newline-separator

Just pass the -n or --newline-separator option and you're good to go. Remember that even existing `;' will be turned into newlines, so your lines may still be messed up at some places. Same for newlines inside expressions like tables, etc. (and without counting the fact that comments are stripped, too)

Comments I intend to put in the PR but that I'll write here until I can actually send the PR:

In luamin.js I'm extending temporary dicts with the new variable baseOptions which contains the newlineSeparator option. I'm not sure if it's the right way to do it, since if the temporary dict contains a newlineSeparator key itself (e.g. later we want to override the behaviour for some kinds of expressions), the baseOptions will override it (which is counter-intuitive for a base option).

This can be solved by either adding a newlineSeparatorOverride option to distinguish keys (more verbose), or by reversing the extend arguments order (baseOptions first), and replacing and using some extendCopy function instead of extend to create a 3rd dict rather than working on the 1st dict in-place (that would mess up the existing baseOptions which is passed by reference).