Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lua: include lpeg module #7649

Merged
merged 2 commits into from
Nov 5, 2021
Merged

Lua: include lpeg module #7649

merged 2 commits into from
Nov 5, 2021

Conversation

tarleb
Copy link
Collaborator

@tarleb tarleb commented Oct 31, 2021

Compiles the 'lpeg' library (Parsing Expression Grammars For Lua) into
the program.

Package maintainers may choose to rely on package dependencies to make
lpeg available, in which case they can compile the with the constraint
lpeg +rely-on-shared-lpeg-library.

@tarleb
Copy link
Collaborator Author

tarleb commented Oct 31, 2021

I seemed to remember this being a request at some point, but I couldn't find the proper issue. I'll publish the lpeg package on Hackage if you think this is a good idea.

@tarleb tarleb changed the title WIP: Lua: include lpeg module Lua: include lpeg module Oct 31, 2021
@tarleb
Copy link
Collaborator Author

tarleb commented Oct 31, 2021

Figured it wouldn't hurt to publish that package, so the PR is now ready as is.

Compiles the 'lpeg' library (Parsing Expression Grammars For Lua) into
the program.

Package maintainers may choose to rely on package dependencies to make
lpeg available, in which case they can compile the with the constraint
`lpeg +rely-on-shared-lpeg-library`.
@jgm
Copy link
Owner

jgm commented Oct 31, 2021

I think I was the one who originally raised this -- because I love lpeg so much, and this would make it easy to create custom parsers.

On the other hand, it's not terribly hard to install lpeg using luarocks and use it in your filters. I guess the advantage of this is that it allows filters written using lpeg to be used on any system with pandoc, without special installation. Maybe it should be discussed on pandoc-discuss?

I imagine this adds very little to the size of the compiled binary?

@tarleb
Copy link
Collaborator Author

tarleb commented Nov 1, 2021

I imagine this adds very little to the size of the compiled binary?

Comparing optimized builds on Ubuntu after stripping, the difference in binary size is 41 KB (0.23‰ of the current size). But it's still an additional dependency.

I'll follow your advice and bring it up on pandoc-discuss.

@jgm
Copy link
Owner

jgm commented Nov 1, 2021

I suppose this could open up a path to allowing custom readers (the way we now have custom writers). These would use the existing pandoc Lua module to create AST elements, but the parsing would be completely up to the writer. If lpeg could be used, this could even be pleasant.

@jgm
Copy link
Owner

jgm commented Nov 3, 2021

There seems to be some enthusiasm for this, and not too much cost, so I'd say let's do it.

@jgm
Copy link
Owner

jgm commented Nov 3, 2021

Interface for custom readers could be something like this:

function Reader(input, reader_options)
  -- parse input into blocks, meta
  -- use lpeg for parsing and pandoc.XXX functions to create AST elements
  return pandoc.Pandoc(blocks, meta)
end

Example application: Some have requested a really simple plain text reader that just does word and paragraph splitting. That would be dead easy to implement with a custom reader along these lines. People with different splitting needs could customize as they see fit.

@jgm jgm merged commit a1b6bf6 into jgm:master Nov 5, 2021
@jgm
Copy link
Owner

jgm commented Nov 5, 2021

@tarleb does one need to do

local lpeg = require 'lpeg'

to use this? (And if one does that, will it use the baked in version only if there is no lpeg in the normal lua search path?)
Could we make it loaded automatically as lpeg so this require step could be skipped?

@tarleb
Copy link
Collaborator Author

tarleb commented Nov 5, 2021

Yes, to all of the above. I'll hack something up.

@tarleb tarleb deleted the lpeg branch November 5, 2021 07:27
@tarleb
Copy link
Collaborator Author

tarleb commented Nov 5, 2021

OK, lpeg is now always loaded into the global lpeg. We still fall back to using the default library loading if the package was configured to rely on the system-wide lpeg library.

It's not the most elegant code tbh, but the whole library loading will need an overhaul at some point. Should be fine until then.

@jgm
Copy link
Owner

jgm commented Nov 5, 2021

Is there any documentation for this? We should at least note that they can use lpeg in this way without having it locally installed, and link to the lpeg documentation.

@tarleb
Copy link
Collaborator Author

tarleb commented Nov 5, 2021

I added a short paragraph to the docs and can try to expand it within the next days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants