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

Source tags rendering incorrectly. #1471

Open
tomchristie opened this issue Apr 19, 2018 · 6 comments
Open

Source tags rendering incorrectly. #1471

tomchristie opened this issue Apr 19, 2018 · 6 comments

Comments

@tomchristie
Copy link
Contributor

@tomchristie tomchristie commented Apr 19, 2018

Refs: encode/django-rest-framework#5945

Building against 0.17.3

image

Building against 0.16.3

image

Presumably they're incorrectly being treated as a list even in the single-tag case.

@waylan
Copy link
Member

@waylan waylan commented Apr 19, 2018

This appears to be related to your custom theme and a change in how MkDocs handles Markdown meta-data. Markdown's default is to return raw meta-data such that each value is a list of lines (most of which contain one item). As this doesn't really make sense for MkDocs, in 0.17 MkDocs now joins those items into a string.

However, in your template, you are still treating the value of page.meta.source as a list, iterating over it, and creating a link for each item. Of course, this is creating a separate link for each character in the string. Instead, your template should just create a single link without iteration.

I'm assuming a page would only ever have a single "source" to link to, so the loss of ability to define a list shouldn't be a problem here. The change was made with the understanding that the feature was completely undocumented and mostly unused. @tomchristie I suspect you are the only one who has ever made use of it. The only officially supported meta-data key is template, which MkDocs was already joining into a single string. But even that was completely undocumented until after the change.

@tomchristie
Copy link
Contributor Author

@tomchristie tomchristie commented Apr 19, 2018

I suspect you are the only one who has ever made use of it.

This is very possibly true. 😝

@tomchristie
Copy link
Contributor Author

@tomchristie tomchristie commented Apr 19, 2018

I'm assuming a page would only ever have a single "source" to link to, so the loss of ability to define a list shouldn't be a problem here.

That’s not quite the case for us, no. Occasionally a single docs page will refer to more than one module.

@waylan
Copy link
Member

@waylan waylan commented Apr 19, 2018

Occasionally a single docs page will refer to more than one module.

I'm not quite sure of the best way to support this. The underlying library offers a way to define a "type" for a given key which would convert the raw data to that "type". However, that is not exposed to the MkDocs user.

Truth be told, I would like to switch to using YAML for the meta-data. Of course, YAML has type-support builtin and users already are using YAML for the config file. The problem is that Markdown systems which use YAML traditionally require a YAML deliminator (line of ---) to separate the data from the Markdown text. Therefore, it's not a straightforward switch. Users would need to edit their docs to conform to the new format. And a list would be defined differently as well, requiring an additional edit by users.

@mitya57
Copy link
Contributor

@mitya57 mitya57 commented Dec 15, 2018

However, in your template, you are still treating the value of page.meta.source as a list, iterating over it, and creating a link for each item. Of course, this is creating a separate link for each character in the string. Instead, your template should just create a single link without iteration.

I have just stumbled upon this. But the mkdocs’ documentation suggests that page.meta.source is a list, while it actually isn’t. Quoting the example from the documentation:

{% for filename in page.meta.source %}
  <a class="github" href="https://github.com/.../{{ filename }}">
    <span class="label label-info">{{ filename }}</span>
  </a>
{% endfor %}

Maybe that example should be fixed to replace the {% for %} loop with {% if page.meta.source %}?

Ah, and also the default theme is doing the same. I tried adding a source: line to a file in mkdocs’ own documentation, and it is rendered as a set of single letters.

@waylan
Copy link
Member

@waylan waylan commented Dec 15, 2018

Good catch. I wasn't aware we were expecting lists anywhere internally.

Probably the correct change to the documentation is to update the meta-data in the sample document to use YAML style meta-data. Then the existing template would be correct. Note that the existing behavior of Meta-Data is correctly documented under writing your docs (YAML style retains types, including lists, while MultiMarkdown style returns a single string for each key).

It might also make sense to have the templates check the type of page.meta.source and act accordingly.

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

Successfully merging a pull request may close this issue.

None yet
3 participants