search

fab13n / metalua-parser

luarock packaging of Metalua's parser
48 stars 2 forks source link

A soft dependency on Metalua #1

Closed agladysh closed 10 years ago

agladysh commented 10 years ago

https://github.com/fab13n/metalua-parser/blob/master/metalua/compiler.lua#L138

In a perfect world this probably should go somewhere else.

fab13n commented 10 years ago

You're right, and there also are soft dependencies to metalua.compiler.bytecode. I have no idea yet of any better place, I'm open to suggestions.

I find it really nice, when working with the full package, to have everything in compiler:${ORIGIN_FORMAT}_to_${TARGET_FORMAT}(input); I'd really prefer to keep it that way.

A possibility would be not to provide the offending commands, such as :ast_to_src(ast) or :ast_to_bytecode(ast), in the parser-only package. But then you'd get an error "trying to call a nil value" instead of "module metalua.compiler.bytecode not found", which seems harder to figure out for newcomers. I tend to prefer helpful messages over overzealous modular purity :)

What would you suggest?

agladysh commented 10 years ago

Well, at the very least do a graceful degradation — detect if metalua.package (or whatever) is not available and fail with nicer error message on call. I.e. assert(pcall(require, ...), "compliler:ast_to_src() requires full metalua") or something like this (maybe capture and report proper error message as well — for full diagnostics).

Consider also moving offending functions to metalua itself — adding them to modules with monkey-patching. If it is a good design decision or not is arguable, matter of taste.

In any case, please document this.

agladysh commented 10 years ago

In any case, please document this.

I see this is already documented in the README, so nevermind. But I'd improve the error message.

fab13n commented 10 years ago

OK for graceful messages. My issue with monkey-patching is, errors make no sense when the monkey is missing. Say, you've installed metalua-parser; then forgot that it wasn't the full metalua. 15 days into your project, you successfully generate ASTs, now it's time to convert them back into SRC. You call compiler:ast_to_src(), can't figure out why the function is missing while the compilerinstance is there and (partially) working.

agladysh commented 10 years ago

You can always do compiler:ast_to_src = function() error("available only in full metalua") end.

Otherwise you'll have a circular dependency when Metalua will depend on metalua-parser.