Skip to content

Commit 3f41174

Browse files
authored
Merge pull request #201 from ruby-docx/refactor-paragraph-substitute
Simplify Paragraph#substitute
2 parents c45ba97 + 815d91b commit 3f41174

2 files changed

Lines changed: 54 additions & 32 deletions

File tree

‎lib/docx/containers/paragraph.rb‎

Lines changed: 40 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -88,41 +88,12 @@ def each_text_run
8888
# See https://github.com/ruby-docx/docx/issues/147 and #187
8989
def substitute(pattern, replacement)
9090
search_from = 0
91-
loop do
92-
texts = @node.xpath(TEXT_XPATH).map { |t_node| Elements::Text.new(t_node) }
93-
94-
offsets = []
95-
cursor = 0
96-
texts.each do |text|
97-
offsets << cursor
98-
cursor += text.content.length
99-
end
100-
full_text = texts.map(&:content).join
101-
102-
match = full_text.match(pattern, search_from)
103-
break unless match
104-
break if match.end(0) == match.begin(0) # ignore empty matches
105-
106-
match_start = match.begin(0)
107-
match_end = match.end(0) # exclusive
108-
# the w:t holding the first and the last matched character
109-
first = texts.each_index.find { |i| offsets[i] + texts[i].content.length > match_start }
110-
last = texts.each_index.find { |i| offsets[i] + texts[i].content.length >= match_end }
111-
91+
while (match = find_match(pattern, search_from))
11292
replaced = match[0].sub(pattern, replacement)
113-
head = texts[first].content[0...(match_start - offsets[first])]
114-
tail = texts[last].content[(match_end - offsets[last])..]
115-
if first == last
116-
texts[first].content = head + replaced + tail
117-
else
118-
texts[first].content = head + replaced
119-
texts[(first + 1)...last].each { |text| text.content = '' }
120-
texts[last].content = tail
121-
remove_separators_between(texts[first].node, texts[last].node, TEXT_XPATH, SEPARATOR_XPATH)
122-
end
93+
replace_text_range(match.begin(0)...match.end(0), replaced)
12394

12495
# advance past the inserted replacement so it is not re-matched
125-
search_from = match_start + replaced.length
96+
search_from = match.begin(0) + replaced.length
12697
end
12798
self
12899
end
@@ -181,6 +152,43 @@ def style=(identifier)
181152
SEPARATOR_XPATH = %w[br tab cr].map { |tag| "w:r/w:#{tag}|w:hyperlink/w:r/w:#{tag}" }.join('|')
182153
private_constant :TEXT_XPATH, :SEPARATOR_XPATH
183154

155+
# The paragraph's w:t, in document order; their contents joined make up #text.
156+
def text_nodes
157+
@node.xpath(TEXT_XPATH).map { |t_node| Elements::Text.new(t_node) }
158+
end
159+
160+
# The first match of +pattern+ in the paragraph text at or after +from+,
161+
# or nil when there is none. An empty match also gives nil, so that
162+
# #substitute stops rather than loop on it.
163+
def find_match(pattern, from)
164+
match = text_nodes.map(&:content).join.match(pattern, from)
165+
match unless match.nil? || match[0].empty?
166+
end
167+
168+
# Replace the characters in +range+ of the paragraph text with +string+.
169+
# The string goes into the w:t holding the range's first character; the
170+
# rest of the range is cut from the following w:t, and line breaks and
171+
# tabs inside it are removed.
172+
def replace_text_range(range, string)
173+
texts = text_nodes
174+
# boundaries[i]...boundaries[i + 1] is where texts[i] sits in the text
175+
boundaries = texts.each_with_object([0]) { |text, acc| acc << acc.last + text.content.length }
176+
first = texts.each_index.find { |i| boundaries[i + 1] > range.begin }
177+
last = texts.each_index.find { |i| boundaries[i + 1] >= range.end }
178+
179+
head = texts[first].content[0...(range.begin - boundaries[first])]
180+
tail = texts[last].content[(range.end - boundaries[last])..]
181+
if first == last
182+
texts[first].content = head + string + tail
183+
return
184+
end
185+
186+
texts[first].content = head + string
187+
texts[(first + 1)...last].each { |text| text.content = '' }
188+
texts[last].content = tail
189+
remove_separators_between(texts[first].node, texts[last].node, TEXT_XPATH, SEPARATOR_XPATH)
190+
end
191+
184192
def style_property
185193
properties&.at_xpath('w:pStyle') || properties&.add_child('<w:pStyle/>').first
186194
end

‎spec/docx/containers/paragraph_spec.rb‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
require 'spec_helper'
44
require 'docx'
5+
require 'timeout'
56

67
describe Docx::Elements::Containers::Paragraph do
78
def paragraph(xml, document_properties = {}, document = nil)
@@ -142,6 +143,19 @@ def paragraph(xml, document_properties = {}, document = nil)
142143
expect(p.text).to eq('{{a}}{{a}}')
143144
end
144145

146+
# Without the guard against empty matches this would loop forever, so it
147+
# runs under a timeout to fail instead of hanging.
148+
it 'ignores a pattern that only matches the empty string' do
149+
p = paragraph('<w:p><w:r><w:t>abc</w:t></w:r></w:p>')
150+
151+
Timeout.timeout(5) do
152+
p.substitute(/x*/, '-')
153+
p.substitute(/x*/, '')
154+
end
155+
156+
expect(p.text).to eq('abc')
157+
end
158+
145159
it 'leaves the paragraph untouched when nothing matches' do
146160
expect(split.substitute('{{missing}}', 'x').text).to eq('Hello {{first_name}}!')
147161
end

0 commit comments

Comments
 (0)