search

asciidoctor / jekyll-asciidoc

:syringe: A Jekyll plugin that converts AsciiDoc source files in your site to HTML pages using Asciidoctor.
https://github.com/asciidoctor/jekyll-asciidoc
MIT License
311 stars 78 forks source link

#158 implement yaml page variable merging #261

Open djencks opened 3 years ago

djencks commented 3 years ago

using asciidoctor.merge_attributes key (#158 partial)

mojavelinux commented 3 years ago

I think merge_attributes should only be a list of attribute names. The definition of the attribute itself should still be under the attributes key. Otherwise, the "merge_attributes" is doing double duty and that just feels awkward. In other words, it should look like:

asciidoc:
  attributes:
    page-header:
      foo: bar
      yin: yang
  merge_attributes: [page-header]
mojavelinux commented 3 years ago

...and maybe even mergeable_attributes

djencks commented 3 years ago

I think merged_attributes might be the best option so far. mergeable_attributes to me leaves in doubt whether they will in fact be merged. I'll update the name once we decide what approach to take with the harder stuff.

I've provided 2 more PRs that show 2 ways of implementing your suggestion of having only the attribute name under merge_attributes. One does not provide the _config.yml value to the page, just like this PR, which feels odd for an attribute under attributes. The other provides the stringified value to the page, which feels extremely odd in comparison with the Object values for other attributes attributes. However, in this case, I don't see any other way to tell Asciidoctor that the value can be overriden, as appending '@' to an object doesn't make sense. This implementation is also significantly more complex than the other two, which are roughly comparable. Ideally there would be a way to supply an Object attribute value to Asciidoctor and say that it can be overridden. I believe extensions can get the actual attribute value as an object.

https://github.com/asciidoctor/jekyll-asciidoc/pull/264

https://github.com/asciidoctor/jekyll-asciidoc/pull/265

Another question is where the merge[d]_attributes key should go. I put it under asciidoctor only. I think it's a bit bizarre to have attributes work under both asciidoc and asciidoctor, so I'd rather choose only one for the new key, and since I don't see this behavior necessarily working for other converter implementations I'd prefer asciidoctor.