Skip to content
New issue

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

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

Already on GitHub? # to your account

include-meta LUA filter (updated) #197

Closed
wants to merge 9 commits into from
Closed

Conversation

mfhepp
Copy link

@mfhepp mfhepp commented Nov 5, 2021

This filter addresses #196.

It should be a complete, functional filter for adding meta-data from external YAML/Markdown files with a new 'include-meta' property for the YAML document header.

You can preview it at https://github.com/mfhepp/lua-filters/tree/master/include-meta.

The merge mechanism is heavily inspired by code contributed by @tarleb in jgm/pandoc#3115 (comment).

Note that this is my very first contribution in LUA, so any ideas for improvement are very welcome!

@tarleb
Copy link
Member

tarleb commented Nov 5, 2021

Thank you, looks good!

Some notes:

  • The example that contains multiple include-meta fields is, as far as I know, not valid YAML. It would fail with pandoc 2.15 and earlier, as those versions were using a stricter library for YAML parsing.
  • Some lines in the README are very long. We generally try to keep lines below 80 characters. Depending on which editor you use, editorconfig might be helpful to assist with these details.
  • Are the tests in test.sh the same as those run with make test? I'm not sure if we need that file.

@mfhepp
Copy link
Author

mfhepp commented Nov 5, 2021

Thanks for the quick review!

Some notes:

  • Some lines in the README are very long. We generally try to keep lines below 80 characters. Depending on which editor you use, editorconfig might be helpful to assist with these details.
    Fixed in the next commit.
  • Are the tests in test.sh the same as those run with make test? I'm not sure if we need that file.
    Yes, this was before I knew that a Makefile is a requirement. Removed.

I also spotted a bug when using the filter with no include-meta directive.

The behavior for RevealJS properties is only as expected if a value that should replace an earlier value is set to 0 for properties that default to true. But I assume this is more a Pandoc issue than related to my LUA filter.

@mfhepp mfhepp changed the title include-meta LUA filter include-meta LUA filter (updated) Nov 5, 2021
@mfhepp
Copy link
Author

mfhepp commented Feb 23, 2022

While I think the filter is pretty mature, I am not pursuing it any further. The reason is that the same effect can be realized with the standard Pandoc mechanism of "default" files with the --defaults=filename.yaml command-line option.

Inside the defaults file, you can specify meta-data defaults, either with the metadata: keyword, the metadata-file: keyword, or the variable: keyword.

So by simply invoking Pandoc with one or more defaults files, one can predefine defaults for meta-data with ease. I do not think it makes sense to develop a filter that does almost the same with slight variants in behavior (e.g. regarding priority rules).

One hint: meta-data values that are path names with underscores (like my_logo.png) cause trouble in some LaTex templates (like in Eisvogel). When you use the variables: keyword in a defaults file, the path will not be parsed as Markdown and those problems will disappear. The same holds for other special characters that Pandoc will escape when it parses them as Markdown.

Same for header-includes sections.

Asking for trouble:

metadata:
    titlepage-logo: "foo_doo_baa/my_logo.pdf"
    header-includes:
        - \usepackage[section]{placeins}

Robust Approach:

variables:
    titlepage-logo: "foo_doo_baa/my_logo.pdf"
    header-includes:
        - \usepackage[section]{placeins}

I think using a defaults.yaml file is more elegant than using the syntax for raw content, like

logo: "`logo_image.pdf`{=latex}"

because this will not work in a metadata value in a defaults file.

See also related issues:

@mfhepp mfhepp closed this Feb 23, 2022
@tarleb
Copy link
Member

tarleb commented Feb 24, 2022

That's a great explanation; I agree completely.

Thank you for all the effort, sorry we didn't get merge it.

@cagix
Copy link

cagix commented Feb 24, 2022

like this idea, thanks!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants