8000 Add clarity to docs on permalinks placeholders and builtins by EmilyGraceSeville7cf · Pull Request #8995 · jekyll/jekyll · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add clarity to docs on permalinks placeholders and builtins #8995

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Apr 8, 2022
Merged

Add clarity to docs on permalinks placeholders and builtins #8995

merged 7 commits into from
Apr 8, 2022

Conversation

EmilyGraceSeville7cf
Copy link
Contributor
@EmilyGraceSeville7cf EmilyGraceSeville7cf commented Mar 14, 2022 •
edited by ashmaroli
Loading

Summary

  • Describe :output_ext as a placeholder
  • Add note on W prefix to :week when using weekdate builtin

Context

Questions

What to do with date and pretty formats now? I want fix documentation errors because it's hard to write JSON schema when they are exist.

Fixing "Built-in formats":
f0f3255 
- global permalink doesn't have "output_ext" placeholder as documentation above tells
- wrong "W:" removed
@EmilyGraceSeville7cf
Copy link
Contributor Author
EmilyGraceSeville7cf commented Mar 14, 2022 •
edited
Loading

This is my attempt to write regex for:

  • global permalink: ^((/(:(year|short_year|month|i_month|short_month|long_month|day|i_day|y_day|w_year|week|w_day|short_day|long_day|hour|minute|second|title|slug|categories|slugified_categories))+)+|date|pretty|ordinal|weekdate|none)$
  • collection permalink: ^(/(:(collection|path|name|title|output_ext))+)+$

mattr-
mattr- previously approved these changes Mar 17, 2022
View reviewed changes
Copy link
Member
@mattr- mattr- left a comment

Choose a reason for hiding this comment

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

looks good to me

@mattr-
Copy link
Member
mattr- commented Mar 17, 2022

I went to go double check the code and now I'm confused. The documentation mirrors the code exactly. When you use these permalink styles on a site, are you seeing different results?

@mattr- mattr- dismissed their stale review March 17, 2022 19:23

It's my review

@EmilyGraceSeville7cf
Copy link
Contributor Author
EmilyGraceSeville7cf commented Mar 18, 2022 •
edited
Loading

  1. There is an example: permalink: /:categories/:year/:month/:day/:title:output_ext. But in this placeholder list no output_ext is listed.

  2. Here is an example: /:categories/:year/W:week/:short_day/:title:output_ext but my regex can't catch W: and rejects this value. What is W:? I found no information about is.

🤔

ashmaroli
ashmaroli previously requested changes Mar 21, 2022
View reviewed changes
Copy link
Member
@ashmaroli ashmaroli left a comment

Choose a reason for hiding this comment

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

Blocking this since docs should mirror Ruby codebase.

@ashmaroli
Copy link
Member
  • There is an example: permalink: /:categories/:year/:month/:day/:title:output_ext. But in this placeholder list no output_ext is listed.

Agreed. But it is mentioned under subsequent section towards end of page
Perhaps this pull request may add the missing placeholder to the global table..

  • Here is an example: /:categories/:year/W:week/:short_day/:title:output_ext but my regex can't catch W: and rejects this value. What is W:? I found no information about is.

W will act as a prefix to placeholder :week.
For example, W10..

Agreed that it has not been documented anywhere. Perhaps this pull request could add that info..

@EmilyGraceSeville7cf
Copy link
Contributor Author

W will act as a prefix to placeholder :week.

What placeholder prefixes are allowed?

@ashmaroli
Copy link
Member

What placeholder prefixes are allowed?

@EmilySeville7cfg See underlying code for reference:

jekyll/lib/jekyll/configuration.rb

Lines 272 to 278 in 19868d5

STYLE_TO_PERMALINK = {
:none => "/:categories/:title:output_ext",
:date => "/:categories/:year/:month/:day/:title:output_ext",
:ordinal => "/:categories/:year/:y_day/:title:output_ext",
:pretty => "/:categories/:year/:month/:day/:title/",
:weekdate => "/:categories/:year/W:week/:short_day/:title:output_ext",
}.freeze

Built-ins are syntactic-sugars (or shorthands) to a particular implementation.
So, weekdate is a shorthand to a particular template: /:categories/:year/W:week/:short_day/:title:output_ext, i.e. no other prefix supported. If one does not want that W, they'll have to manually write the template without the W.

P.S. You may add a note to reduce confusion.. Also, please update the PR title and description to better describe latest diff.

@EmilyGraceSeville7cf EmilyGraceSeville7cf changed the title Fixing "Built-in formats" Mention "output_ext" in "Placeholders" and note about "W" prefix Apr 3, 2022
ashmaroli
ashmaroli reviewed Apr 3, 2022
View reviewed changes
Emily Grace Seville and others added 2 commits April 4, 2022 01:46
@ashmaroli
Apply suggestion from code review
5fdfddd 
Co-authored-by: Ashwin Maroli <ashmaroli@users.noreply.github.com>
@ashmaroli
Markup note about prefix W in HTML
97603dc
@ashmaroli ashmaroli changed the title Mention "output_ext" in "Placeholders" and note about "W" prefix Add clarity to docs on permalinks placeholders and builtins Apr 3, 2022
@ashmaroli ashmaroli dismissed their stale review April 3, 2022 16:33

Concerns addressed

@ashmaroli
Copy link
Member

Thanks @EmilySeville7cfg
@jekyllbot: merge +doc

@jekyllbot jekyllbot merged commit 64dbf46 into jekyll:master Apr 8, 2022
jekyllbot added a commit that referenced this pull request Apr 8, 2022
@jekyllbot
Update history to reflect merge of #8995 [ci skip]
c254533
github-actions bot pushed a commit that referenced this pull request Apr 8, 2022
@jekyllbot
Deploy docs from 64dbf46
30276a5 
Emily Grace Seville: Add clarity to docs on permalinks placeholders and builtins (#8995)

Merge pull request 8995
@jekyll jekyll locked and limited conversation to collaborators Apr 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ashmaroli ashmaroli ashmaroli approved these changes

@mattr- mattr- mattr- left review comments

Assignees
No one assigned
Labels
documentation frozen-due-to-age
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@EmilyGraceSeville7cf @mattr- @ashmaroli @jekyllbot
0