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

Perform details matching on UnboundTemplates #42004

Merged
merged 1 commit into from
Apr 21, 2021

Conversation

jhawthorn
Copy link
Member

@jhawthorn jhawthorn commented Apr 16, 2021

In old versions of Rails, we would rely entirely on what was returned by Dir.glob to determine the match and sorting of our templates.

Later we switched to building a regex on each search, which allowed us to perform a much faster glob, find matching templates with the regex, and then emulate the sort order based on captures from the regex.

Now we have PathParser, which can parse any template's details accurately from just its filename (not depending on the query being made).

This PR moves the matching to done on UnboundTemplates, effectively using details found by the PathParser for both matching and sorting of templates, and removing the dynamic regex for queries.

This should be faster at boot/after reloads as we're no longer building a regex and additionally we only need to parse a template's path for details one time (we can use the same details for matching/sorting in future queries with different details).

Copy link
Contributor

@kaspth kaspth left a comment

Choose a reason for hiding this comment

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

Some tricky stuff in here! But seems ok from what I can spot.

In old versions of Rails, we would rely entirely on what was returned by
Dir.glob to determine the match and sorting of our templates.

Later we switched to building a regex on each search, which allowed us
to perform a much faster glob, find matching templates with the regex,
and then emulate the sort order based on captures from the regex.

Now we have PathParser, which can parse any template's details
accurately from just its filename (not depending on the query being
made).

This commit moves the matching to done on UnboundTemplates, effectively
using details found by the PathParser for both matching and sorting of
templates, and removing the dynamic regex for queries.

This should be faster at boot/after reloads as we're no longer building
a regex and additionally we only need to parse a template's path for
details one time (we can use the same details for matching/sorting in
future queries with different details).
@jhawthorn jhawthorn marked this pull request as ready for review April 20, 2021 21:59
@jhawthorn jhawthorn merged commit 53a5fe4 into rails:main Apr 21, 2021
ricardotk002 added a commit to ricardotk002/rails that referenced this pull request Apr 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants