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

Update include tag to be more permissive #8618

Merged
merged 3 commits into from
Mar 27, 2021

Conversation

parkr
Copy link
Member

@parkr parkr commented Mar 26, 2021

Summary

Our {% include %} tag presently isn't permissive enough to handle many valid filenames, include foo@bar.html. This @ symbol is used by systems within the Node ecosystem and the include tag is often used to selectively include HTML/JS/CSS from within a Node package in a Jekyll site.

This does not include any of the reserved characters: https://en.wikipedia.org/wiki/Filename#Reserved_characters_and_words

It adds support for the following characters:

  • @
  • #
  • ~
  • +
  • -
  • ( and )

Context

This PR continues the work that @Convincible did in #7096.

We'd like to be able to use this in GitHub Pages, so I'll plan a back port to Jekyll 3.x as well.

@parkr parkr requested review from a team, mattr- and ashmaroli and removed request for a team March 26, 2021 19:49
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.

I'm fine with the additions as is, though I agree about both { and } being on the bubble for inclusion.

Thanks for prepping tests for this! ❤️

@parkr
Copy link
Member Author

parkr commented Mar 26, 2021

@mattr- Thank you! I removed [ ] { and } for now. Easier to add them later than to try to remove them.

@parkr
Copy link
Member Author

parkr commented Mar 26, 2021

**Continuous Integration / Style Check (Ruby 2.5) (pull_request) ** Failing after 4m — Style Check (Ruby 2.5)

@ashmaroli Hi! Are these style checks failing on master? I am happy to fix them up but I didn't change anything about these failures in my file that I noticed... Thanks!!

@DirtyF
Copy link
Member

DirtyF commented Mar 27, 2021

@parkr I fixed the styles for Rubocop 1.12

@DirtyF
Copy link
Member

DirtyF commented Mar 27, 2021

@jekyll: merge +minor

@jekyllbot jekyllbot merged commit 5d01099 into master Mar 27, 2021
@jekyllbot jekyllbot deleted the update-include-tag-to-be-more-permissive branch March 27, 2021 15:36
jekyllbot added a commit that referenced this pull request Mar 27, 2021
github-actions bot pushed a commit that referenced this pull request Mar 27, 2021
Parker Moore: Update include tag to be more permissive (#8618)

Merge pull request 8618
@parkr
Copy link
Member Author

parkr commented Mar 28, 2021

@DirtyF i can't remember - is there an easy way to backport this to 3.x? I can't remember if @jekyllbot can do it for us or not.

@DirtyF
Copy link
Member

DirtyF commented Mar 29, 2021

@parkr I never done a backport but according to parkr/auto-reply#31 @jekyllbot can't do this.

There's a script/backport-pr in Jekyll's repository though

parkr added a commit that referenced this pull request Apr 6, 2021
parkr added a commit that referenced this pull request Apr 6, 2021
parkr added a commit that referenced this pull request Apr 7, 2021
Backport #8618 for v3.9.x: Update include tag to be more permissive
@parkr parkr mentioned this pull request Apr 8, 2021
6 tasks
@jekyll jekyll locked and limited conversation to collaborators Mar 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants