-
Notifications
You must be signed in to change notification settings - Fork 285
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
Conversation
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
I'd probably extend 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. |
I agree with @palant, it would be better if we extend the existing |
I suggest adding htmlUnescape to title and rawTitle. |
There was a problem hiding this 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 -}} |
There was a problem hiding this comment.
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?
@palant My apologies, I merged this in a hurry. Could you create another pull request to improve this? |
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>
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?