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

Remove unnecessary markdownify calls #646

Closed
mpourismaiel opened this issue Dec 3, 2019 · 3 comments · Fixed by #667
Closed

Remove unnecessary markdownify calls #646

mpourismaiel opened this issue Dec 3, 2019 · 3 comments · Fixed by #667
Assignees
Milestone

Comments

@mpourismaiel
Copy link
Member

Is this a BUG REPORT or FEATURE REQUEST?:
Refactor Suggestion

Explanation:
We changed markdown configuration within exampleSite's config.toml file to add support for Hugo v0.60 and the new GoldMark markdown compiler but the solution is not really good. It's much better to remove any unnecessary markdownify calls which causes problems for rendering with the unsafe html errors.

@Marzal
Copy link
Contributor

Marzal commented Dec 4, 2019

Hi, this is what I have tested using syna/layouts/partials/fragments/footer.html

This is the code used right now (with a bit of DEBUG):

<div class="col-md m-2
  {{- partial "helpers/text-color.html" (dict "self" $self.self "light" "secondary") -}}
">
  {{- with .self.Content }}
    <div>DEBUG.self.Content
      {{- . | markdownify -}}
    </div>
  {{- end }}
  DEBUG.self.Content
  {{- .self.Content -}}
</div>

With Goldmark gives:

    <div>DEBUG.self.Content
<!-- raw HTML omitted --></div>

  DEBUG.self.Content
<h4 id="syna-theme">Syna Theme</h4>
<p>Highly customizable open source theme for Hugo based static websites</p>

With blackfriday gives what you expect:

    <div>DEBUG.self.Content
<h4 id="syna-theme">Syna Theme</h4>
<p>Highly customizable open source theme for Hugo based static websites</div>

  DEBUG.self.Content
<h4 id="syna-theme">Syna Theme</h4>
<p>Highly customizable open source theme for Hugo based static websites</p>

Using markdownify only gives one change, we lose the closing </p>, and it doesn't work with Goldmark by default


Now if we change self.Content to self.RawContent when we are using markdownify:

<div class="col-md m-2
  {{- partial "helpers/text-color.html" (dict "self" $self.self "light" "secondary") -}}
">
  {{- with .self.RawContent }}
    <div>DEBUG.self.RawContent
      {{- . | markdownify -}}
    </div>
  {{- end }}
  DEBUG.self.Content
  {{- .self.Content -}}
</div>

With Goldmark gives:

    <div>DEBUG.self.RawContent
<h4 id="syna-theme">Syna Theme</h4>
<p>Highly customizable open source theme for Hugo based static websites</div>

  DEBUG.self.Content
<h4 id="syna-theme">Syna Theme</h4>
<p>Highly customizable open source theme for Hugo based static websites</p>
</div>

With blackfriday gives the same result

    <div>DEBUG.self.RawContent
<h4 id="syna-theme">Syna Theme</h4>
<p>Highly customizable open source theme for Hugo based static websites</div>

  DEBUG.self.Content
<h4 id="syna-theme">Syna Theme</h4>
<p>Highly customizable open source theme for Hugo based static websites</p>
</div>

So I think it is safe to assume that self.Content has already the markdown processed to html and that's why Goldmark fail with markdownify.
It seems like markdownify is redundant, no need to convert again the html code in self.Content, and not allowed by Goldmark by default. Also without markdownify you have the closing </p>

I can't test this with an older version of Hugo (0.60.1 right now) to see if self.Content was always already rendered making markdownify redundant.

But I'm going to try to get the full Public folder with the code as it is, then remove all | markdownify with sed to get a new Public and compare both. Using blackfriday and goldmark (with unsafe = true).

@stp-ip
Copy link
Member

stp-ip commented Dec 5, 2019

Awesome analysis and debugging. Seems like "markdownify" might be a left-over artifact from early versions, which isn't necessary anymore. Much appreciated. Looking forward to the PR resolving compatibility for 0.60.1 \o/

@mpourismaiel
Copy link
Member Author

@Marzal Thanks again for your help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

3 participants