styles icon indicating copy to clipboard operation
styles copied to clipboard

Use Jekyll/Liquid include for images/figures instead of Markdown

Open rgaiacs opened this issue 7 years ago • 14 comments

At the moment we are using "standard" Markdown syntax to include images/figures. Markdown syntax lacks some features such as caption. I want to propose we start using Jekyll/Liquid include for the next release.

Current behaviour

$ cat image.md 
![foo](foo.png 'foo text description')
$ kramdown image.md 
<p><img src="foo.png" alt="foo" title="foo text description" /></p>

Propose behaviour

$ cat image.md 
{% include figure.html filename="foo.png" caption="Caption describing foo" alternative_text="foo text description" %}
$ kramdown image.md 
<figure>
  <img
  src="foo.png"
  alt="foo text description">	
  <figcaption>Caption describing foo</figcaption>
</figure>

Required parameters

  • filename
  • alternative_text

Optional parameters

  • caption
  • license, preferable CC-BY
  • source_url, for any content that we didn't authored

Backward compatibility

This will not break backward compatibility. Any lesson that didn't migrate to use include will not see any difference.

rgaiacs avatar Aug 01 '17 11:08 rgaiacs

Can Kramdown parse these includes to give us a list of figures? If not, how will we regenerate _includes/all_figures.html ?

gvwilson avatar Aug 01 '17 13:08 gvwilson

Can Kramdown parse these includes to give us a list of figures?

Jekyll uses Liquid for all the tags. We could change https://github.com/swcarpentry/lesson-example/blob/gh-pages/bin/markdown_ast.rb to do the tag processing replacement, see https://github.com/Shopify/liquid/wiki/Liquid-for-Programmers, before kramdown do their job.

rgaiacs avatar Aug 01 '17 14:08 rgaiacs

how will we regenerate _includes/all_figures.html ?

I usually deal with this in a liquid loop. Keep a yml file in _data (e.g. _data/figures.yml ) and then loop over them in the html :

{% for figure in site.data.figures %}
<img src="{{ figure.url }}" alt={{ figure.alt }}">
{% endfor %}

Forgive me if I'm misunderstanding something, but isn't this what you're looking for ?

brucellino avatar Aug 01 '17 15:08 brucellino

@brucellino we don't keep a list of figures on any file because if authors need to edit more than one place we have a point of failure on our workflow. For that reason, we need a script to collect all the figures "in the correct order".

rgaiacs avatar Aug 01 '17 16:08 rgaiacs

Ok (sorry for barging in on this - I thought I had a good suggestion !). So, if I understand it correctly, you just want to be able to have the added features by including an image template (figures.html), the issue is not separating content from form. In that case, (and just for argument's sake) why not just write the image bits in html ? I can see that you want to avoid asking people to write in a format they are not comfortable with.

brucellino avatar Aug 01 '17 16:08 brucellino

why not just write the image bits in html ?

Because this is error prone in the long run. Is the same reason why programmers write functions instead of duplicate code, if they need to change something they only need to change once. And as we need to eat our own dog food, using the template is the correct way. To give you a better example, if today we use CSS framework X and our HTML need to be

<figure class="x">
  <img
  src="foo.png"
  alt="foo text description">	
  <figcaption>Caption describing foo</figcaption>
</figure>

but tomorrow we use framework Y and our HTML need to be

<figure class="y">
  <img
  src="foo.png"
  alt="foo text description">	
  <figcaption>Caption describing foo</figcaption>
</figure>

we only need to make the change in one place, the template, instead of going in all the files looking for where we need to make the change.

rgaiacs avatar Aug 01 '17 18:08 rgaiacs

Just to clarify @gvwilson, currently the _includes/all_figures.html is generated by the Makefile using the extract_figures.py. The advantage of that method is that it is grabbing all the figs in order and generating the all_figures. The disadvantage is that it is not part of the Jekyll build, so doesn't show up while developing with Jekyll.

We can reproduce the all_figures page with Liquid, including getting images in the correct order, by replacing _includes/all_figures.html with something like:

{% for item in site.episodes %}
{% assign lines = item.content | newline_to_br | split:'<br />' %}
{% for line in lines %}
{% if line contains '<img' %}
{{ line }}
{% endif %}
{% endfor %}
{% endfor %}

I like this sort of solution because it relies on one less python file, and is part of the Jekyll build process which makes development easier. It would work with both the current standard markdown images and with @rgaiacs figure include (assuming the img tag is on one line, rather than broken up like he displays it above), because it grabs the site.episodes after the .md files have been processed (i.e. they are converted to html and have their includes pulled in).

evanwill avatar Aug 02 '17 00:08 evanwill

@evanwill Thanks for your feedback.

We can reproduce the all_figures page with Liquid, including getting images in the correct order, by replacing _includes/all_figures.html with something like: (...)

We can't use your code snippet because, as you mention on your comment, it will fail when we have

<img
src="foo.jpg"
>

which is a valid HTML code. This is the main reason that we are using the abstract syntax tree (AST) provided by kramdown, the same parser used by Jekyll.

Since most of the comments to my proposal so far diverted to the "List of Figures", I created another proposal, see #162, to discuss it.

rgaiacs avatar Aug 02 '17 07:08 rgaiacs

I think this needs a cost-benefit analysis.

Markdown is easy to write. It's what we currently use and it works fine.

While image captions might be useful in some places, I don't feel that learning the new syntax and changing all the code is worth while. Some images will probably get missed or not work properly and someone will have to fix it.

gcapes avatar Aug 02 '17 08:08 gcapes

@rgaiacs actually my Liquid snippet above will catch your figure include, even with the extra white space, since Liquid is looking at the page.content object in Jekyll, which standardizes the HTML since everything goes through the rendering engine. If you write a _include/figure.html like you suggest above, something like:

<figure>
  <img 
  src="{{site.url}}/fig/{{ include.file }}" 
  alt="{{ include.alt }}" >
  <figcaption>{{ include.caption }}</figcaption>
</figure>

And use it in a page like: {% include figure.html file="forking.svg" alt="cat pic" caption="My cat" %}

It will render without the extra white space, like:

<figure>
  <img src="http://localhost:4000/fig/forking.svg" alt="cat" />
  <figcaption>My cat</figcaption>
</figure>

Also, if the user writes some non-standard img tags into a markdown episode, it will still catch it, because kramdown normalizes the white space and tags.

My take is that a Jekyll project is a closed loop. You create the includes and set a few conventions, you don't need to parse for every possibility.

By adding a _include/figure.html you provide another option, but doesn't take away the current method of doing things. No one has to learn a new syntax unless they want to easily add a caption. The only objection was that the figure include doesn't work with the extract_figures.py to create all_figures.html. My Liquid proposal is simpler and actually more robust than the python script, since it is looking at the rendered html of the episodes.

evanwill avatar Aug 02 '17 17:08 evanwill

@evanwill Thanks very much for all the clarification. 💖

rgaiacs avatar Aug 03 '17 06:08 rgaiacs

ha ha, actually didn't mean to strongly argue for it, but I am working on some jekyll projects right now and have a sort of crush on Liquid.

In any case, I agree that having a _include/figure.html is helpful, and I see it on many other projects at the moment. Simplicity of writing and adapting the lesson content seems most important. Currently, adapting the styles and lesson-example repos seems overly complex. However, if we aren't making bigger changes to organizing the code and workflow, then introducing details like a fig include probably isn't necessary.

evanwill avatar Aug 03 '17 07:08 evanwill

I'm -1 on this. Anything that deviates from Markdown syntax makes contributing more difficult, and reduces interoperability of the content.

fmichonneau avatar Aug 20 '17 14:08 fmichonneau

After talk with other instructions we will merge this after a review of https://github.com/swcarpentry/styles/blob/gh-pages/bin/lesson_check.py and the documentation.

rgaiacs avatar Sep 09 '17 13:09 rgaiacs