Ignore empty statements handling in Lua 5.1

ghost commented 7 years ago

Well, Lua 5.1 doesn't support empty statements, as said in the reference manual: https://www.lua.org/manual/5.1/manual.html#2.4.1

fstirlitz commented 7 years ago

My remarks:

Don't make multiple unrelated changes in a single commit, and don't make fix-up commits; squash them instead. I see later commits undoing the addition of a licence block and making other changes; please squash and force-push these four commits. It's easier to review with related changes in a single commit.
The only sort-of descriptive commit message misuses ignore to mean disable. Vocabulary aside, it probably wouldn't hurt to write something longer. Maybe not as verbose as cd7247510ed7e4db741eb1f8562307e3080ff31d, but something providing more context, at least with a link to the Lua manual.
Please add some test cases. You can look into tests/scaffolding/operators to see how they are made; add something similar to tests/scaffolding/statements, then run make scaffold-tests in the root directory of the project.
It's bad form for code with side effects to rely on short-circuiting of Boolean operators (except to prevent an out-of-bounds exception). It's a bug waiting to happen when someone overlooks the condition or forgets about short-circuiting. Please revert to the if statement form.

ghost commented 7 years ago

@fstirlitz Hm, okay. I'll elaborate a new commit, but... how do I set the Lua version in the test cases? Edit: Got it!

fstirlitz commented 7 years ago

You could just force-push on this branch. No need to close the pull request.

ghost commented 7 years ago

@fstirlitz I'm not sure what is force-push, I'm kinda new in GitHun... t-t

ghost commented 7 years ago

@fstirlitz Unfortunately make scaffold-tests isn't a valid command.

fstirlitz / luaparse