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

Change regex to sanitize and normalize filenames passed to LiquidRenderer #6610

Merged
merged 2 commits into from Mar 10, 2018

Conversation

ashmaroli
Copy link
Member

@ashmaroli ashmaroli commented Dec 8, 2017

Motivation:

Currently, the resulting table from building with --profile has relative_path for files at the source dir but an absolute_path for files within a theme-gem (which can be a long string depending on the user's Ruby Installation location.)

Patch notes:

  • all filenames are normalized to be relative to the source-directory or theme-root.
  • if a path starts with a non-alphanumeric char, (e.g. ../../ or ~foo), those characters are ignored.
  • no discernible effect on performance.

UPDATE - 09 FEB 2018

  • prepend relative paths from theme-gem with the gem's dirname
    (e.g. minima-2.3.0/_includes/head.html)

@ashmaroli ashmaroli added this to the v3.8.0 milestone Feb 4, 2018
@DirtyF
Copy link
Member

DirtyF commented Feb 9, 2018

current display when building with minima:

Filename                                              | Count |  Bytes |  Time
------------------------------------------------------+-------+--------+------
_posts/2018-01-22-welcome-to-jekyll.markdown          |     1 |  1.52K | 0.429
_layouts/default.html                                 |     4 | 20.80K | 0.095
_includes/head.html                                   |     4 |  7.25K | 0.056
_includes/footer.html                                 |     4 |  3.82K | 0.019
feed.xml                                              |     1 |  3.62K | 0.019
_includes/header.html                                 |     4 |  5.38K | 0.015
_includes/social.html                                 |     4 |  0.27K | 0.013
_layouts/post.html                                    |     1 |  2.20K | 0.003
_layouts/home.html                                    |     1 |  0.44K | 0.001
_layouts/page.html                                    |     1 |  0.57K | 0.000
_posts/2018-01-22-welcome-to-jekyll.markdown/#excerpt |     1 |  0.42K | 0.000
about.md                                              |     1 |  0.36K | 0.000
404.html                                              |     1 |  0.37K | 0.000
assets/main.scss                                      |     1 |  0.02K | 0.000
index.md                                              |     1 |  0.00K | 0.000

@ashmaroli
Copy link
Member Author

ashmaroli commented Feb 9, 2018

current display when building with minima

Are you sure you have pulled the most recent commit, the output looks as expected on Travis

Filename                                              | Count |  Bytes |  Time
------------------------------------------------------+-------+--------+------
_posts/2018-02-09-welcome-to-jekyll.markdown          |     1 |  1.52K | 0.352
_layouts/default.html                                 |     4 | 22.34K | 0.089
minima-2.3.0/_includes/head.html                      |     4 |  7.26K | 0.049
feed.xml                                              |     1 |  3.62K | 0.019
minima-2.3.0/_includes/header.html                    |     4 |  5.38K | 0.018
minima-2.3.0/_includes/footer.html                    |     4 |  5.25K | 0.017
minima-2.3.0/_includes/social.html                    |     4 |  1.70K | 0.012
_layouts/home.html                                    |     1 |  0.46K | 0.003
_layouts/post.html                                    |     1 |  2.20K | 0.002
_layouts/page.html                                    |     1 |  0.67K | 0.000
_posts/2018-02-09-welcome-to-jekyll.markdown/#excerpt |     1 |  0.42K | 0.000
about.md                                              |     1 |  0.47K | 0.000
404.html                                              |     1 |  0.36K | 0.000
assets/main.scss                                      |     1 |  0.02K | 0.000
index.md                                              |     1 |  0.00K | 0.000

@ghost ghost mentioned this pull request Feb 17, 2018
13 tasks
@DirtyF DirtyF requested a review from a team March 5, 2018 22:46
Copy link
Member

@DirtyF DirtyF left a comment

Choose a reason for hiding this comment

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

LGTM

@DirtyF
Copy link
Member

DirtyF commented Mar 10, 2018

@jekyllbot: merge +minor

@jekyllbot jekyllbot merged commit e1b64f9 into jekyll:master Mar 10, 2018
jekyllbot added a commit that referenced this pull request Mar 10, 2018
@jekyll jekyll locked and limited conversation to collaborators Jul 12, 2019
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

3 participants