pandoc / lua-filters

A collection of lua filters for pandoc
MIT License
603 stars 165 forks source link

Minted filter for beamer and latex, safe for all writers #40

Closed svenevs closed 5 years ago

svenevs commented 5 years ago

Hey! I made a new years resolution to get this all nice and shiny and in lua, and I think it's looking pretty good so far.

jgm commented 5 years ago

Stephen McDowell notifications@github.com writes:

  • -- NOTE: using "latex" here because using FORMAT as a parameter does
  • -- not seem to work. It should not cause problems for beamer.
  • return pandoc.RawBlock("latex", raw_minted)

Sorry, this one occurs in two places and I missed both in the last review. Why can't I do return pandoc.RawBlock(FORMAT, raw_minted)? If I do that then nothing shows up (aka the code blocks just disappear).

That's a bit of a rough edge in pandoc. "beamer" doesn't currently work as a raw format. I've just pushed a commit fixing that, but for now use "latex".

svenevs commented 5 years ago

Ok the proposed final documentation is rendered here. I just need to add some tests later tonight and then it should be good (I'll add a new comment and remove [WIP] from title when ready for final review).

When we think this is ready for merge, is it ok if I squash these all into one commit? The commit messages aren't very good, and the diff stack is unnecessary (I had some back and forth ...). I hesitate to do this because when I do I think GitHub removes the review comments with the questions I have above. But if they aren't actually problems, then I'll force push the badness away :)

tarleb commented 5 years ago

Great! Feel free to squash, but we can also squash this when merging (we generally do this for all PRs introducing new filters).

svenevs commented 5 years ago

ok i think this looks good :slightly_smiling_face:

i hope the tests being written in python is ok, i had to do a lot of string manipulation and that's definitely my go-to language for that. grepping for the latex was annoying because latex syntax definitely conflicts with regex syntax.

sorry for multiple force pushes :D but now it's just one commit, unless things need to change in which case future commits will be structured better and can just be squash merged on your end.

svenevs commented 5 years ago

Ok round two complete, thanks for the really helpful review on making this much cleaner @tarleb! All of your feedback has been incorporated and a few updates to the tests to actually validate the Header (ensure_not_present(test_name, "fragile", output)) in the non-beamer stuff, and make sure metadata doesn't produce nil. I think this looks really good now :slightly_smiling_face:

IIRC when squashed the title of the PR is what the main commit message is. I don't know what the main message should be, but feel free to edit it to be whatever you want.

tarleb commented 5 years ago

Thanks again! This is probably the best documented and tested filter in the collection.

svenevs commented 5 years ago

Woohoo, you're very welcome! Hehe yes ... I got a little carried a way xD

mfhepp commented 2 years ago

Am impressed by how well this filter is written and documented! Dedication to detail at its best :-)!

I hope this is the best place to leave this info for others: For the ArXiv preprints workflow, there are some limitations on using minted. In essence, it is important to use a non-hidden cache directory, like so

\usepackage[frozencache=true,cachedir=minted-cache]{minted}

The important thing is that the cachedir name does not start with a dot as the default .minted-cache.

Full details are here: https://arxiv.org/help/faq/mistakes#minted

Disclaimer: I did not test this yet, but thought it is worth logging it for the future.

@svenevs @tarleb