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

Projects
None yet
4 participants
@ashmaroli
Member

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 the profiling label Dec 13, 2017

@ashmaroli ashmaroli added this to the v3.8.0 milestone Feb 4, 2018

@DirtyF

This comment has been minimized.

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

This comment has been minimized.

Member

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

@oe oe referenced this pull request Feb 17, 2018

Closed

Release 3.8.0 #6783

12 of 13 tasks complete

@DirtyF DirtyF requested a review from jekyll/core Mar 5, 2018

@DirtyF

DirtyF approved these changes Mar 8, 2018

LGTM

@oe

oe approved these changes Mar 9, 2018

@DirtyF

This comment has been minimized.

Member

DirtyF commented Mar 10, 2018

@jekyllbot: merge +minor

@jekyllbot jekyllbot merged commit e1b64f9 into jekyll:master Mar 10, 2018

3 checks passed

WIP ready for review
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment