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

Add mode parameter to slugify Liquid filter #2918

Merged
merged 3 commits into from Jan 18, 2015

Conversation

Projects
None yet
6 participants
@nitoyon
Contributor

nitoyon commented Sep 13, 2014

Add mode parameter to slugify Liquid filter. The default value is default.

SLUGIFY STYLE DESCRIPTION
none Convert to lowercase.
raw Convert to lowercase, and replace every sequence of spaces with a hyphen.
default Convert to lowercase, and replace every sequence of spaces and non-alphanumeric characters with a hyphen.
pretty Convert to lowercase, and replace every sequence of spaces and non-alphanumeric characters (except for ._~!$&'()+,;=@) with a hyphen.

Examples:

{{ "The _config.yml file?" | slugify }}
#=> the-config-yml-file

{{ "The _config.yml file?" | slugify:'none' }}
#=> the _config.yml file?

{{ "The _config.yml file?" | slugify:'raw' }}
#=> the-_config.yml-file?

{{ "The _config.yml file?" | slugify:'default' }}
#=> the-config-yml-file

{{ "The _config.yml file?" | slugify:'pretty' }}
#=> the-_config.yml-file
@kansaichris

This comment has been minimized.

Show comment
Hide comment
@kansaichris

kansaichris Sep 14, 2014

This is actually something I thought about when I added the slugify filter—are there any special characters (other than hyphens) that we could or should allow in permalink slugs by default?

Incidentally, I don't think it would be a good idea to include a '#' sign in permalinks—wouldn't that be interpreted as a named anchor?

kansaichris commented Sep 14, 2014

This is actually something I thought about when I added the slugify filter—are there any special characters (other than hyphens) that we could or should allow in permalink slugs by default?

Incidentally, I don't think it would be a good idea to include a '#' sign in permalinks—wouldn't that be interpreted as a named anchor?

@nitoyon

This comment has been minimized.

Show comment
Hide comment
@nitoyon

nitoyon Sep 14, 2014

Contributor

are there any special characters (other than hyphens) that we could or should allow in permalink slugs by default?

I think we could allow ._~!$&'()+,;=@, because

  • -._~!$&'()*+,;=:@ is not escaped in URI path segment (see Jekyll::URL::escape_path for detail)
  • \/:*?"<>| is reserved in NTFS (although Windows is not supported officially)

Incidentally, I don't think it would be a good idea to include a '#' sign in permalinks—wouldn't that be interpreted as a named anchor?

We can use # sign for filename of post and page (e.g. _posts/2014-09-14-c#.md is written to _site/2014/09/14/c#/index.html and page.url is set to /2014/09/14/c%23/).

So, even if we include a # sign for a document, it should be work. When I create a document named _albums/c#.md in v2.4, it is written to albums/c%23/index.html (albums/c#/index.html is expected!!) and doc.url is set to albums/c%23/ (no problem). This should be fixed soon.

Contributor

nitoyon commented Sep 14, 2014

are there any special characters (other than hyphens) that we could or should allow in permalink slugs by default?

I think we could allow ._~!$&'()+,;=@, because

  • -._~!$&'()*+,;=:@ is not escaped in URI path segment (see Jekyll::URL::escape_path for detail)
  • \/:*?"<>| is reserved in NTFS (although Windows is not supported officially)

Incidentally, I don't think it would be a good idea to include a '#' sign in permalinks—wouldn't that be interpreted as a named anchor?

We can use # sign for filename of post and page (e.g. _posts/2014-09-14-c#.md is written to _site/2014/09/14/c#/index.html and page.url is set to /2014/09/14/c%23/).

So, even if we include a # sign for a document, it should be work. When I create a document named _albums/c#.md in v2.4, it is written to albums/c%23/index.html (albums/c#/index.html is expected!!) and doc.url is set to albums/c%23/ (no problem). This should be fixed soon.

@nitoyon

This comment has been minimized.

Show comment
Hide comment
@nitoyon

nitoyon Sep 14, 2014

Contributor

So, even if we include a # sign for a document, it should be work. When I create a document named _albums/c#.md in v2.4, it is written to albums/c%23/index.html (albums/c#/index.html is expected!!) and doc.url is set to albums/c%23/ (no problem). This should be fixed soon.

Fix this bug in #2924

Contributor

nitoyon commented Sep 14, 2014

So, even if we include a # sign for a document, it should be work. When I create a document named _albums/c#.md in v2.4, it is written to albums/c%23/index.html (albums/c#/index.html is expected!!) and doc.url is set to albums/c%23/ (no problem). This should be fixed soon.

Fix this bug in #2924

@kansaichris

This comment has been minimized.

Show comment
Hide comment
@kansaichris

kansaichris Sep 15, 2014

We can use # sign for filename of post and page (e.g. _posts/2014-09-14-c#.md is written to _site/2014/09/14/c#/index.html and page.url is set to /2014/09/14/c%23/).

Oh, I see—# will work as long as it is escaped in URLs. Do we really want the slugify filter to generate permalinks with escaped characters, though? I thought the point was to generate "clean" permalinks...

@parkr, what do you think?

kansaichris commented Sep 15, 2014

We can use # sign for filename of post and page (e.g. _posts/2014-09-14-c#.md is written to _site/2014/09/14/c#/index.html and page.url is set to /2014/09/14/c%23/).

Oh, I see—# will work as long as it is escaped in URLs. Do we really want the slugify filter to generate permalinks with escaped characters, though? I thought the point was to generate "clean" permalinks...

@parkr, what do you think?

@alfredxing

This comment has been minimized.

Show comment
Hide comment
@alfredxing

alfredxing Sep 15, 2014

Member

I don't think slugify should generate escaped characters, since the whole point of slugify is to generate a human-readable (pretty) URL slug. Ideally it would generate something guessable (so for the c# example, c-sharp would be optimal).

Member

alfredxing commented Sep 15, 2014

I don't think slugify should generate escaped characters, since the whole point of slugify is to generate a human-readable (pretty) URL slug. Ideally it would generate something guessable (so for the c# example, c-sharp would be optimal).

@nitoyon

This comment has been minimized.

Show comment
Hide comment
@nitoyon

nitoyon Sep 17, 2014

Contributor

How about adding slugify option to _config.yml?

This option affects how slugify filter works and how a document is slugified.

For example:

SLUGIFY STYLE DESCRIPTION The _config.yml file? is converted to
slugify: normal No sign except for '-' the-config-yml-file
slugify: pretty No URI escape the-_config.yml-file
slugify: raw ' ''-' / lower case the-_config.yml-file? (URL: the-_config.yml-file%3F)
slugify: none Do nothing The _config.yml file? (URL: The%20_config.yml%20file%3F)

slugify: normal is the default value for backward compatibility.

Contributor

nitoyon commented Sep 17, 2014

How about adding slugify option to _config.yml?

This option affects how slugify filter works and how a document is slugified.

For example:

SLUGIFY STYLE DESCRIPTION The _config.yml file? is converted to
slugify: normal No sign except for '-' the-config-yml-file
slugify: pretty No URI escape the-_config.yml-file
slugify: raw ' ''-' / lower case the-_config.yml-file? (URL: the-_config.yml-file%3F)
slugify: none Do nothing The _config.yml file? (URL: The%20_config.yml%20file%3F)

slugify: normal is the default value for backward compatibility.

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Sep 17, 2014

Member

The 🎄 of configuration options doesn't need another ornament. I'd be 👎 to the slugify config option.

What I want to see is a backwards-compatible solution. We have already released slugify so we can't change its default behaviour until 3.0. Allowing some additional argument to the Liquid filter is OK.

Member

parkr commented Sep 17, 2014

The 🎄 of configuration options doesn't need another ornament. I'd be 👎 to the slugify config option.

What I want to see is a backwards-compatible solution. We have already released slugify so we can't change its default behaviour until 3.0. Allowing some additional argument to the Liquid filter is OK.

@nitoyon

This comment has been minimized.

Show comment
Hide comment
@nitoyon

nitoyon Sep 19, 2014

Contributor

Adding configuration option doesn't change the default behaviour. slugify filter works like v2.4.0 as long as slugify option is not set or is set to the default value.

If slugify option is added to _config.yml,

  • we can change how document's title is slugified to URL (e.g. the-config-yml-file or the-_config.yml-file).
  • in the future, jekyll-archive plugin could use this option to determine how tag name and category is converted to URL (e.g. We can configure archive page of Node.js tag to be located at /tag/node-js/ or /tag/node.js/ or /tag/Node.js/). It looks bad if jekyll-archive has its own slugify implementation. It would be cool if jekyll-archive uses Jekyll::Utils::slugify and slugify option.
Contributor

nitoyon commented Sep 19, 2014

Adding configuration option doesn't change the default behaviour. slugify filter works like v2.4.0 as long as slugify option is not set or is set to the default value.

If slugify option is added to _config.yml,

  • we can change how document's title is slugified to URL (e.g. the-config-yml-file or the-_config.yml-file).
  • in the future, jekyll-archive plugin could use this option to determine how tag name and category is converted to URL (e.g. We can configure archive page of Node.js tag to be located at /tag/node-js/ or /tag/node.js/ or /tag/Node.js/). It looks bad if jekyll-archive has its own slugify implementation. It would be cool if jekyll-archive uses Jekyll::Utils::slugify and slugify option.
@nitoyon

This comment has been minimized.

Show comment
Hide comment
@nitoyon

nitoyon Sep 19, 2014

Contributor

In this PR, I won't add new configuration options. I'll create new PR if necessary.

I'm going to fix this PR in order to add raw mode. Wait a moment please.

Contributor

nitoyon commented Sep 19, 2014

In this PR, I won't add new configuration options. I'll create new PR if necessary.

I'm going to fix this PR in order to add raw mode. Wait a moment please.

@nitoyon nitoyon changed the title from Add allowed_chars parameter to slugify Liquid filter to Add option parameter to slugify Liquid filter Sep 22, 2014

@nitoyon

This comment has been minimized.

Show comment
Hide comment
@nitoyon

nitoyon Sep 28, 2014

Contributor

Changed parameter specifications (See the first comment for detail).

Contributor

nitoyon commented Sep 28, 2014

Changed parameter specifications (See the first comment for detail).

Show outdated Hide outdated lib/jekyll/filters.rb
Show outdated Hide outdated lib/jekyll/filters.rb
Show outdated Hide outdated lib/jekyll/utils.rb
Show outdated Hide outdated lib/jekyll/utils.rb
Show outdated Hide outdated site/_docs/templates.md
Show outdated Hide outdated site/_docs/templates.md
Show outdated Hide outdated test/test_filters.rb
Show outdated Hide outdated lib/jekyll/utils.rb

@nitoyon nitoyon changed the title from Add option parameter to slugify Liquid filter to Add mode parameter to slugify Liquid filter Oct 11, 2014

@nitoyon

This comment has been minimized.

Show comment
Hide comment
@nitoyon

nitoyon Oct 11, 2014

Contributor

Fixed & rebased 😃

Contributor

nitoyon commented Oct 11, 2014

Fixed & rebased 😃

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jan 17, 2015

Member

Would you please rebase this, @nitoyon? I like it.

Member

parkr commented Jan 17, 2015

Would you please rebase this, @nitoyon? I like it.

@parkr parkr merged commit ae57f69 into jekyll:master Jan 18, 2015

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

parkr added a commit that referenced this pull request Jan 18, 2015

@jekyll jekyll locked and limited conversation to collaborators Feb 27, 2017

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