Skip to content

Commit ad8487d

Browse files
committed
Refactor Scanner logic out of AroundBlockScan introduce history
AroundBlockScan started as a utility class that was meant to be used as a DSL for scanning and making new blocks. As logic got added to this class it became hard to reason about what exactly is being mutated when. I pulled the scanning logic out into it's own class which gives us a clean separation of concerns. This allowed me to remove a lot of accessors that aren't core to the logic provided by AroundBlockScan. In addition to this refactor the ScanHistory class can snapshot a scan. This allows us to be more aggressive with scans in the future as we can now snapshot and rollback if it didn't turn out the way we wanted. The change comes with a minor perf impact: before: 5.092678 0.104299 5.196977 ( 5.226494) after: 5.128536 0.099871 5.228407 ( 5.249542) This represents a 0.996x change in speed (where 1x would be no change and 2x would be twice as fast). This is a 0.38% decrease in performance which is negligible. It's likely coming from the extra blocks being created while scanning. This is negligible and if the history feature works well we might be able to make better block decisions which is means fewer calls to ripper which is the biggest bottleneck. While this doesn't fix #187 it's a good intermediate step that will hopefully make working on that issue easier.
1 parent 6fda3f6 commit ad8487d

File tree

7 files changed

+327
-155
lines changed

7 files changed

+327
-155
lines changed
Lines changed: 135 additions & 147 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
# frozen_string_literal: true
22

3+
require_relative "scan_history"
4+
35
module SyntaxSuggest
46
# This class is useful for exploring contents before and after
57
# a block
@@ -24,22 +26,17 @@ module SyntaxSuggest
2426
# puts scan.before_index # => 0
2527
# puts scan.after_index # => 3
2628
#
27-
# Contents can also be filtered using AroundBlockScan#skip
28-
#
29-
# To grab the next surrounding indentation use AroundBlockScan#scan_adjacent_indent
3029
class AroundBlockScan
3130
def initialize(code_lines:, block:)
3231
@code_lines = code_lines
33-
@orig_before_index = block.lines.first.index
34-
@orig_after_index = block.lines.last.index
3532
@orig_indent = block.current_indent
36-
@skip_array = []
37-
@after_array = []
38-
@before_array = []
39-
@stop_after_kw = false
4033

41-
@force_add_hidden = false
34+
@stop_after_kw = false
4235
@force_add_empty = false
36+
@force_add_hidden = false
37+
@target_indent = nil
38+
39+
@scanner = ScanHistory.new(code_lines: code_lines, block: block)
4340
end
4441

4542
# When using this flag, `scan_while` will
@@ -89,47 +86,34 @@ def stop_after_kw
8986
# stopping if we've found a keyword/end mis-match in one direction
9087
# or the other.
9188
def scan_while
92-
stop_next = false
93-
kw_count = 0
94-
end_count = 0
95-
index = before_lines.reverse_each.take_while do |line|
96-
next false if stop_next
97-
next true if @force_add_hidden && line.hidden?
98-
next true if @force_add_empty && line.empty?
89+
stop_next_up = false
90+
stop_next_down = false
9991

100-
kw_count += 1 if line.is_kw?
101-
end_count += 1 if line.is_end?
102-
if @stop_after_kw && kw_count > end_count
103-
stop_next = true
104-
end
92+
@scanner.scan(
93+
up: ->(line, kw_count, end_count) {
94+
next false if stop_next_up
95+
next true if @force_add_hidden && line.hidden?
96+
next true if @force_add_empty && line.empty?
10597

106-
yield line
107-
end.last&.index
98+
if @stop_after_kw && kw_count > end_count
99+
stop_next_up = true
100+
end
108101

109-
if index && index < before_index
110-
@before_index = index
111-
end
112-
113-
stop_next = false
114-
kw_count = 0
115-
end_count = 0
116-
index = after_lines.take_while do |line|
117-
next false if stop_next
118-
next true if @force_add_hidden && line.hidden?
119-
next true if @force_add_empty && line.empty?
102+
yield line
103+
},
104+
down: ->(line, kw_count, end_count) {
105+
next false if stop_next_down
106+
next true if @force_add_hidden && line.hidden?
107+
next true if @force_add_empty && line.empty?
120108

121-
kw_count += 1 if line.is_kw?
122-
end_count += 1 if line.is_end?
123-
if @stop_after_kw && end_count > kw_count
124-
stop_next = true
125-
end
109+
if @stop_after_kw && end_count > kw_count
110+
stop_next_down = true
111+
end
126112

127-
yield line
128-
end.last&.index
113+
yield line
114+
}
115+
)
129116

130-
if index && index > after_index
131-
@after_index = index
132-
end
133117
self
134118
end
135119

@@ -160,79 +144,104 @@ def scan_while
160144
# 4 def eat
161145
# 5 end
162146
#
163-
def capture_neighbor_context
147+
def capture_before_after_kws
164148
lines = []
165-
kw_count = 0
166-
end_count = 0
167-
before_lines.reverse_each do |line|
168-
next if line.empty?
169-
break if line.indent < @orig_indent
170-
next if line.indent != @orig_indent
171-
172-
kw_count += 1 if line.is_kw?
173-
end_count += 1 if line.is_end?
174-
if kw_count != 0 && kw_count == end_count
175-
lines << line
176-
break
177-
end
178-
179-
lines << line if line.is_kw? || line.is_end?
180-
end
181-
182-
lines.reverse!
183-
184-
kw_count = 0
185-
end_count = 0
186-
after_lines.each do |line|
187-
next if line.empty?
188-
break if line.indent < @orig_indent
189-
next if line.indent != @orig_indent
190-
191-
kw_count += 1 if line.is_kw?
192-
end_count += 1 if line.is_end?
193-
if kw_count != 0 && kw_count == end_count
194-
lines << line
195-
break
196-
end
197-
198-
lines << line if line.is_kw? || line.is_end?
199-
end
149+
up_stop_next = false
150+
down_stop_next = false
151+
@scanner.commit_if_changed
200152

153+
lines = []
154+
@scanner.scan(
155+
up: ->(line, kw_count, end_count) {
156+
break if up_stop_next
157+
next true if line.empty?
158+
break if line.indent < @orig_indent
159+
next true if line.indent != @orig_indent
160+
161+
# If we're going up and have one complete kw/end pair, stop
162+
if kw_count != 0 && kw_count == end_count
163+
lines << line
164+
break
165+
end
166+
167+
lines << line if line.is_kw? || line.is_end?
168+
},
169+
down: ->(line, kw_count, end_count) {
170+
break if down_stop_next
171+
next true if line.empty?
172+
break if line.indent < @orig_indent
173+
next true if line.indent != @orig_indent
174+
175+
# if we're going down and have one complete kw/end pair,stop
176+
if kw_count != 0 && kw_count == end_count
177+
lines << line
178+
break
179+
end
180+
181+
lines << line if line.is_kw? || line.is_end?
182+
}
183+
)
184+
@scanner.try_rollback
201185
lines
202186
end
203187

204188
# Shows the context around code provided by "falling" indentation
205189
#
206-
# Converts:
207190
#
191+
# If this is the original code lines:
192+
#
193+
# class OH
194+
# def hello
208195
# it "foo" do
196+
# end
197+
# end
209198
#
210-
# into:
199+
# And this is the line that is captured
200+
#
201+
# it "foo" do
202+
#
203+
# It will yield its surrounding context:
211204
#
212205
# class OH
213206
# def hello
214-
# it "foo" do
215207
# end
216208
# end
217209
#
210+
# Example:
211+
#
212+
# AroundBlockScan.new(
213+
# block: block,
214+
# code_lines: @code_lines
215+
# ).on_falling_indent do |line|
216+
# @lines_to_output << line
217+
# end
218+
#
218219
def on_falling_indent
219-
last_indent = @orig_indent
220-
before_lines.reverse_each do |line|
221-
next if line.empty?
222-
if line.indent < last_indent
223-
yield line
224-
last_indent = line.indent
225-
end
226-
end
227-
228-
last_indent = @orig_indent
229-
after_lines.each do |line|
230-
next if line.empty?
231-
if line.indent < last_indent
232-
yield line
233-
last_indent = line.indent
234-
end
235-
end
220+
last_indent_up = @orig_indent
221+
last_indent_down = @orig_indent
222+
223+
@scanner.commit_if_changed
224+
@scanner.scan(
225+
up: ->(line, _, _) {
226+
next true if line.empty?
227+
228+
if line.indent < last_indent_up
229+
yield line
230+
last_indent_up = line.indent
231+
end
232+
true
233+
},
234+
down: ->(line, _, _) {
235+
next true if line.empty?
236+
if line.indent < last_indent_down
237+
yield line
238+
last_indent_down = line.indent
239+
end
240+
true
241+
}
242+
)
243+
@scanner.try_rollback
244+
self
236245
end
237246

238247
# Scanning is intentionally conservative because
@@ -267,38 +276,33 @@ def lookahead_balance_one_line
267276
return self if kw_count == end_count # nothing to balance
268277

269278
# More ends than keywords, check if we can balance expanding up
279+
next_up = @scanner.next_up
280+
next_down = @scanner.next_down
270281
if (end_count - kw_count) == 1 && next_up
271-
return self unless next_up.is_kw?
272-
return self unless next_up.indent >= @orig_indent
273-
274-
@before_index = next_up.index
282+
if next_up.is_kw? && next_up.indent >= @orig_indent
283+
@scanner.scan(
284+
up: ->(line, _, _) { line == next_up },
285+
down: ->(line, _, _) { false }
286+
)
287+
end
275288

276289
# More keywords than ends, check if we can balance by expanding down
277290
elsif (kw_count - end_count) == 1 && next_down
278-
return self unless next_down.is_end?
279-
return self unless next_down.indent >= @orig_indent
280-
281-
@after_index = next_down.index
291+
if next_down.is_end? && next_down.indent >= @orig_indent
292+
@scanner.scan(
293+
up: ->(line, _, _) { false },
294+
down: ->(line) { line == next_down }
295+
)
296+
end
282297
end
283298
self
284299
end
285300

286301
# Finds code lines at the same or greater indentation and adds them
287302
# to the block
288303
def scan_neighbors_not_empty
289-
scan_while { |line| line.not_empty? && line.indent >= @orig_indent }
290-
end
291-
292-
# Returns the next line to be scanned above the current block.
293-
# Returns `nil` if at the top of the document already
294-
def next_up
295-
@code_lines[before_index.pred]
296-
end
297-
298-
# Returns the next line to be scanned below the current block.
299-
# Returns `nil` if at the bottom of the document already
300-
def next_down
301-
@code_lines[after_index.next]
304+
@target_indent = @orig_indent
305+
scan_while { |line| line.not_empty? && line.indent >= @target_indent }
302306
end
303307

304308
# Scan blocks based on indentation of next line above/below block
@@ -310,11 +314,12 @@ def next_down
310314
# the `def/end` lines surrounding a method.
311315
def scan_adjacent_indent
312316
before_after_indent = []
313-
before_after_indent << (next_up&.indent || 0)
314-
before_after_indent << (next_down&.indent || 0)
315317

316-
indent = before_after_indent.min
317-
scan_while { |line| line.not_empty? && line.indent >= indent }
318+
before_after_indent << (@scanner.next_up&.indent || 0)
319+
before_after_indent << (@scanner.next_down&.indent || 0)
320+
321+
@target_indent = before_after_indent.min
322+
scan_while { |line| line.not_empty? && line.indent >= @target_indent }
318323

319324
self
320325
end
@@ -331,29 +336,12 @@ def code_block
331336
# Returns the lines matched by the current scan as an
332337
# array of CodeLines
333338
def lines
334-
@code_lines[before_index..after_index]
335-
end
336-
337-
# Gives the index of the first line currently scanned
338-
def before_index
339-
@before_index ||= @orig_before_index
340-
end
341-
342-
# Gives the index of the last line currently scanned
343-
def after_index
344-
@after_index ||= @orig_after_index
345-
end
346-
347-
# Returns an array of all the CodeLines that exist before
348-
# the currently scanned block
349-
private def before_lines
350-
@code_lines[0...before_index] || []
339+
@scanner.lines
351340
end
352341

353-
# Returns an array of all the CodeLines that exist after
354-
# the currently scanned block
355-
private def after_lines
356-
@code_lines[after_index.next..-1] || []
342+
# Managable rspec errors
343+
def inspect
344+
"#<#{self.class}:0x0000123843lol >"
357345
end
358346
end
359347
end

lib/syntax_suggest/block_expand.rb

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -125,17 +125,20 @@ def expand_indent(block)
125125
#
126126
# We try to resolve this edge case with `lookahead_balance_one_line` below.
127127
def expand_neighbors(block)
128-
neighbors = AroundBlockScan.new(code_lines: @code_lines, block: block)
128+
now = AroundBlockScan.new(code_lines: @code_lines, block: block)
129+
130+
# Initial scan
131+
now
129132
.force_add_hidden
130133
.stop_after_kw
131134
.scan_neighbors_not_empty
132135

133136
# Slurp up empties
134-
with_empties = neighbors
137+
now
135138
.scan_while { |line| line.empty? }
136139

137140
# If next line is kw and it will balance us, take it
138-
expanded_lines = with_empties
141+
expanded_lines = now
139142
.lookahead_balance_one_line
140143
.lines
141144

0 commit comments

Comments
 (0)