Simplify output path generation #1829
Conversation
The twisted way to create a javac like output path breaks many use cases, including very simple ones with lexer + parser grammar in a relative subdir (where parser generation doesn't find the lexer tokens). Additionally, not all projects prefer to have their generated files in an output folder which is comprised of the given output path and the subdir given for the grammar files. The check for absolute paths as decision criterion is a weak one anyway - it makes simple usage difficult and doesn't deal with path expansion on all platforms. If a javac like output path is required it should be easy to construct this upfront and provide it as the output path parameter, instead of implicitely creating it. This patch simplifies things without any WTF moments because generated files end up and unexpected locations. It now can deal with simple cases (no output, no grammar subdir) up to complicated cases (nested and/or absolute output dir, multiple grammar subfolders).
Removed some duplicate classes there too.
Also made converter local vars in conversion routines, instead of static global vars.
|
Is it possible to finally merge this patch? I unfortunately did the changes on the master branch and now I cannot open any pull request without the changes from this patch. But since I merged my master already into several other branches (partially also with a pull request) it's impossible to fix this situation without this patch being merged. I'm sorry for this hassle - I should have done the patch on a different branch. However, Travis CI seems ok with the changes (Appveyor obviously has build problems), so I think it is safe to merge. As I said before, relative paths don't work anyway currently (which was the reason I actually rewrote that part) and absolute paths are not affected by this patch. Also the sheer list of bugs that get fixed by this patch should be a good reason to merge. I also explained in e.g. bug #638 why other solutions don't work. |
|
OK, doesn't seem to be so "impossible" as I thought :-) I reverted the path changes in master and will do a separate pull request for them. Sorry for confusion. |
The twisted way to create a javac like output path breaks many use cases, including very simple ones with lexer + parser grammar in a relative subdir (where parser generation doesn't find the lexer tokens). Additionally, not all projects prefer to have their generated files in an output folder which is comprised of the given output path and the subdir given for the grammar files. The check for absolute paths as decision criterion is a weak one anyway - it makes simple usage difficult and doesn't deal with path expansion on all platforms.
If a javac like output path is required it should be easy to construct this upfront and provide it as the output path parameter, instead of implicitely creating it. This patch simplifies things without any WTF moments because generated files end up at unexpected locations. It now can deal with simple cases (no output, no grammar subdir) up to complicated cases (nested and/or absolute output dir, multiple grammar subfolders).
If more than one output folder is wanted run the tool multiple times with different output path settings and add the lib option if you need to reference previously created files.
It might look like a disadvantage that now generation of parser files ends up at a different path when run with relative grammar subfolders, but actually that was broken anyway so I don't expect anyone having used that (but instead either absolute paths or no subfolders at all).
Fixes #1087, #753, #638, tunnelvisionlabs/antlr4ts#306, tunnelvisionlabs/antlr4ts#303