-
-
Notifications
You must be signed in to change notification settings - Fork 13
feat(tags): slugify tag names #164
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
Conversation
88a1b76 to
98693ae
Compare
|
Mostly looks good. Since the permalink is basically the real identifier of a tag, what happens if you have both the I think my intuition says that currently this would treat them as different, but the one that gets processed second will overwrite the first in the file system. |
|
I think you're correct, but can do some verification this evening. This could be made certain by actually making the permalink the key in the |
|
Your intuition is correct. The current solution is problematic because it shows two separate tags, but they both render to the same filename overwriting each other, so only one of the two actually shows a linked page. I have a fix that I'm adding as a second commit that resolves this by having a two-stage map to prevent an external API change. Instead of mapping There's a display oddity where the display value (
The display value will depend on the file sort order. If Orozco is processed first, then the tag on the tag page will be displayed as I'm using a clean commit but am happy to squash it down if you prefer. |
|
Does it make more sense to URI encode the tag names for the permalink rather than turning them into slugs? that way you can make them whatever you want without worrying that they'll merge or overwrite another. |
|
I think that the slug approach is more natural given that this is for the web, and spaces in URLs are poorly handled in general—even when URI-encoded. The disconnect comes from what you want to see and what you want to Just Work regardless of your hosting solution. |
|
Okay, i've done some research, and I think this is a reasonable approach. One thing I'd like to add and I think is doable in this PR, is to make it so you can hard code a tags slug. I believe this is best done in the TagExtension's config, something like config :tableau, Tableau.TagExtension,
tags: %{
"C++" => [slug: "c-plus-plus"],
}If a tag is not configured, it just does the default behavior. |
|
Sure. I'll see about doing that a bit later. One question: should we "lint" the configured tags to ensure there's no spaces or do we make it buyer-beware? |
|
Buyer beware for now. Seems like what the other projects do. |
2607606 to
7c438c4
Compare
This change makes tags with spaces or special characters play nicely
with the Web. By default, tag display values will be converted to
`slugs` using [Slugify][slugify] when building permalinks.
The tag extension configuration has been extended to have a `:tags`
option with a map of display values to options (the only supported
option is `:slug`):
```elixir
config :tableau, Tableau.TagExtension,
tags: %{
"C++" => [slug: "c-plus-plus"],
}
```
This will be used instead of automatic slug conversion. Automatic slug
conversion can be configured in the main Tableau configuration with the
`:slug` keyword option, which is passed directly to [Slugify][slugify].
Other extension writers may use `Tableau.Extension.Common.slugify/2`.
[slugify]: https://hexdocs.pm/slugify/Slug.html
Resolves: elixir-tools#160
7c438c4 to
f380a4c
Compare
This change makes tags with spaces or special characters play nicely with the Web. By default, tag display values will be converted to
slugsusing Slugify when building permalinks.The tag extension configuration has been extended to have a
:tagsoption with a map of display values to options (the only supported option is:slug):This will be used instead of automatic slug conversion. Automatic slug conversion can be configured in the main Tableau configuration with the
:slugkeyword option, which is passed directly to Slugify.Other extension writers may use
Tableau.Extension.Common.slugify/2.Resolves: #160