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

First approach to markdownify title #183

Merged
merged 3 commits into from
Jun 2, 2020
Merged

Conversation

Syralist
Copy link
Contributor

This is my approach to use markdownified titles. Implements #179
Not sure if this is the best way to do it, or if there is a more central place to apply markdownify just once?

Squashed commit of the following:

commit 3761c94
Author: Thomas Helmke <thomas.helmke@gmx.de>
Date:   Wed May 27 20:09:02 2020 +0200

    fixed missing newline

commit b923140
Author: Thomas Helmke <thomas.helmke@gmx.de>
Date:   Wed May 27 20:05:37 2020 +0200

    fixed missing changes from upstream

commit bfd2815
Author: Thomas Helmke <thomas.helmke@gmx.de>
Date:   Sat Apr 4 18:16:41 2020 +0200

    added hashtags to share-urls.

commit 4fbd48d
Author: Thomas Helmke <thomas.helmke@gmx.de>
Date:   Wed May 27 19:44:33 2020 +0200

    first approach to markdownify title

commit 819f500
Author: reuixiy <reuixiy@gmail.com>
Date:   Sun Apr 5 19:05:46 2020 +0800

    revert to preserve the capitalization

commit 1f1bbff
Author: reuixiy <reuixiy@gmail.com>
Date:   Sun Apr 5 12:24:34 2020 +0800

    remove `anchorize` function

    Looks like hyphen(-) will make hashtag failed on Twitter.

commit ef479f1
Author: reuixiy <reuixiy@gmail.com>
Date:   Sun Apr 5 12:06:25 2020 +0800

    use `anchorize` function

commit 4ee9402
Author: reuixiy <reuixiy@gmail.com>
Date:   Sun Apr 5 11:55:10 2020 +0800

    add hyphens to trim the whitespace

commit f021177
Author: reuixiy <reuixiy@gmail.com>
Date:   Sun Apr 5 11:53:12 2020 +0800

    move position

commit 1b5c4a1
Author: reuixiy <reuixiy@gmail.com>
Date:   Sun Apr 5 11:48:27 2020 +0800

    remove `addHashtags` param

commit 1916eda
Author: Thomas Helmke <thomas.helmke@gmx.de>
Date:   Sat Apr 4 18:21:59 2020 +0200

    Delete de-de.toml

commit 4c21588
Author: Thomas Helmke <thomas.helmke@gmx.de>
Date:   Sat Apr 4 18:16:41 2020 +0200

    added hashtags to share-urls.

commit df33eeb
Author: Thomas Helmke <thomas.helmke@gmx.de>
Date:   Fri Mar 6 17:55:30 2020 +0100

    added german translation
@palant
Copy link
Collaborator

palant commented May 28, 2020

I'd probably extend partials/utils/title.html to make it return htmlTitle as well, the logic likely won't stay this simple. In particular, I'd assume that @reuixiy wants to run the titles through partials/utils/markdownify.html rather than plain markdownify, e.g. for sake of emojis. And this functionality should probably be behind a config parameter - and be it so that existing users can leave it off and don't worry about breakage.

The question is whether category/tag titles should receive the same treatment - I'd say that they should, for consistency reasons. This requires changes in a bunch more places.

And you are probably aware of it but just in case: page title is used in some more templates, e.g. partials/components/post-share.html, index.sectionrss.xml and index.sectionatom.xml (these need so start using $rawTitle).

@reuixiy
Copy link
Owner

reuixiy commented May 30, 2020

I agree with @palant, it would be better if we extend the existing partials/utils/title.html.

@reuixiy reuixiy requested a review from palant May 30, 2020 13:08
@Syralist
Copy link
Contributor Author

I suggest adding htmlUnescape to title and rawTitle.
Otherwise there will be things like – in the title. In the browser title and the RSS this would be tolerable but unaesthetic. But in the share-links it breaks the links causing the titles to be abbreviated.

@reuixiy reuixiy merged commit 1322fdd into reuixiy:master Jun 2, 2020
Copy link
Collaborator

@palant palant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry about taking too long with this review, we had holidays here in Germany. This looks good, it should work. Only one suggestion to make the logic easier to follow.

Note however that this is a breaking change - people might have existing titles which won't be left unchanged by the Markdown processor.

@@ -1,27 +1,30 @@
{{- $ := index . "$" -}}
{{- $rawTitle := .title -}}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's rather non-obvious now what this template returns, with the final calculations being performed in the return statement. How about making things less confusing and naming this variable $htmlTitle? $rawTitle can then be defined afterwards as |plainify|htmlUnescape version of it. And $title would be based on $rawTitle again. Or was allowing HTML tags in .Site.Title intentional?

@reuixiy
Copy link
Owner

reuixiy commented Jun 2, 2020

@palant My apologies, I merged this in a hurry. Could you create another pull request to improve this?

ulmefors pushed a commit to ulmefors/hugo-theme-meme that referenced this pull request Nov 22, 2020
closes reuixiy#179

* First approach to markdownify title

* refactor: use title.html

* apply htmlUnescpae to title and rawTitle

Co-authored-by: reuixiy <reuixiy@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants