Support custom YAML parsers

[RiskoZoSlovenska]

An alternate implementation of https://github.com/jgm/lcmark/pull/5.

This PR adds a new yaml_parser option (a function) that, if provided, will be called instead of yaml.load. When not provided, lcmark will try to load the yaml library at runtime and use that.

Since yaml conflicts with most other Lua YAML libraries (lyaml, lua-yaml), this PR also removes it from the rockspec dependency list and adds a note to the README.

[jgm]

Thanks. Is it possible to have bin/lcmark test to see if lyaml is available and, if it is, use it instead of the default yaml?

[RiskoZoSlovenska]

Thanks. Is it possible to have bin/lcmark test to see if lyaml is available and, if it is, use it instead of the default yaml?

Alright, done.

On a related note, I'm starting to think it wouldn't be a bad idea if lcmark itself checked for the presence of other libraries and used whichever one was available. Only downside I can think of is maintainability. What do you think?

[jgm]

On a related note, I'm starting to think it wouldn't be a bad idea if lcmark itself checked for the presence of other libraries and used whichever one was available. Only downside I can think of is maintainability. What do you think?

Hm, not sure. That does seem tempting. As for the maintainability hit, do you mean because people will be running with different yaml libraries, making it harder to deal with bug reports, or because custom code will be required for each supported yaml parser? (If the latter, how much extra code would be needed?)

[RiskoZoSlovenska]

Hm, not sure. That does seem tempting. As for the maintainability hit, do you mean because people will be running with different yaml libraries, making it harder to deal with bug reports, or because custom code will be required for each supported yaml parser? (If the latter, how much extra code would be needed?)

It's not necessarily a lot of code.

Click to expand

```lua local default_yaml_parser = nil local function try_load(module_name, func_name) if default_yaml_parser then -- already loaded; skip return end local success, loaded = pcall(require, module_name) if not success then return end if type(loaded) ~= "table" or type(loaded[func_name]) ~= "function" then return end default_yaml_parser = loaded[func_name] lcmark.yaml_parser_name = module_name .. "." .. func_name end try_load("yaml", "load") -- must come before yaml.eval try_load("yaml", "eval") try_load("lyaml", "load") try_load("ryaml", "decode") try_load("tinyyaml", "parse") -- the reason yaml.load must come before yaml.eval is that the 'yaml' library -- prints error messages if you try to index non-existent fields such as 'eval' -- plus also some minor changes to parse_document_with_metadata ```

The maintainability hit I meant was that this creates some assumptions about how certain libraries work, and if any undergo drastic API changes, this code will have to be updated. Looking at it now, though, it isn't as bad as I had initially feared.

Shall I add this to the PR?

[jgm]

Yeah, that's not too complicated. To reduce maintainability worries, we could cut down on the number of libraries searched...maybe limiting it to the highest quality C parser and the highest quality pure Lua one. And why not try lyaml first, since it's higher quality?

[RiskoZoSlovenska]

Yeah, that's not too complicated. To reduce maintainability worries, we could cut down on the number of libraries searched...maybe limiting it to the highest quality C parser and the highest quality pure Lua one.

So lyaml, lua-yaml and yaml?

And why not try lyaml first, since it's higher quality?

Fair point.

[jgm]

Sounds good to me.