Skip to content

Commit

Permalink
Build symbols descending from stars with regexp
Browse files Browse the repository at this point in the history
Before this commit we initialized all Symbols with the default regexp,
then later on reassigned any symbols descending from stars with either
their regexp from `@requirements` or the default greedy regexp.

With this commit we initialize all Symbols descending from Stars with
the greedy regexp at parse time. This allows us to get rid of the star
branch in path/pattern, since any regexps from `@requirements` will
already have been set in the symbol branch of this code.

This is essentially an alternate version of rails#38901. Getting rid of the
extra branch makes some performance work I am doing a bit easier, plus
it saves us a few method calls. Also the constant saves us from creating
the same regexp multiple times, and it is nice to give that regexp a
name.
  • Loading branch information
composerinteralia committed Jul 25, 2020
1 parent 7556f7c commit 185c4f2
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 26 deletions.
7 changes: 4 additions & 3 deletions actionpack/lib/action_dispatch/journey/nodes/node.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,10 @@ class Symbol < Terminal # :nodoc:
attr_reader :name

DEFAULT_EXP = /[^\.\/\?]+/
def initialize(left)
super
@regexp = DEFAULT_EXP
GREEDY_EXP = /(.+)/
def initialize(left, regexp = DEFAULT_EXP)
super(left)
@regexp = regexp
@name = -left.tr("*:", "")
end

Expand Down
28 changes: 14 additions & 14 deletions actionpack/lib/action_dispatch/journey/parser.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion actionpack/lib/action_dispatch/journey/parser.y
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ rule
| expression OR or { Or.new([val.first, val.last]) }
;
star
: STAR { Star.new(Symbol.new(val.last)) }
: STAR { Star.new(Symbol.new(val.last, Symbol::GREEDY_EXP)) }
;
terminal
: symbol
Expand Down
11 changes: 3 additions & 8 deletions actionpack/lib/action_dispatch/journey/path/pattern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,9 @@ def eager_load!
end

def ast
@spec.each do |node|
if node.symbol?
re = @requirements[node.to_sym]
node.regexp = re if re
elsif node.star?
node = node.left
node.regexp = @requirements[node.to_sym] || /(.+)/
end
@spec.find_all(&:symbol?).each do |node|
re = @requirements[node.to_sym]
node.regexp = re if re
end

@spec
Expand Down

0 comments on commit 185c4f2

Please sign in to comment.