Skip to content

Commit

Permalink
Strip trailing space from the lines of an address
Browse files Browse the repository at this point in the history
There's a feature in the Kramdown parser (i.e. the library that the
Govspeak parser is built on top of) where two or more trailing
spaces at the end of a line of Govspeak will be converted into an
additional line break in the output HTML.

Until recently, the contents of address blocks weren't "reparsed" in the
Govspeak parser, meaning that they weren't subject to this Kramdown
feature. But the Govspeak parser code was recently updated and the
contents of address blocks are now reparsed. This has resulted in
unintentional additional line spacing being made visible in some
addresses that have been rendered by this new version Govspeak.

A typical example of the issue looks like:

| Name
|
| Street
|
| Town
|
| Postcode
|

where it would've previously looked like:

| Name
| Street
| Town
| Postcode

By stripping the trailing space from Govspeak lines within address
blocks, we effectively disable the Kramdown feature and prevent
unintentional line breaks in addresses going forward.

BTW, it's still straightforward to introduce additional line breaks
deliberately.

_A note on the move to handling the last line of the address separately
from the rest:_

In Govspeak, address blocks don't necessarily have a line break between
the last line of the address and its closing tag. Until now, where the
input Govspeak did have a line break, the output HTML replaced it with
a <br>. When there wasn't a line break, the output had no <br>. This
difference didn't seem to me to have any impact on the final rendered
page in the browser, so I've gone ahead and stopped those <br>s from
appearing (as can be seen in the changes we've made here to existing
tests).

So now, while trailing spaces and backslashes are being stripped from
all address lines, in addition to that, for the very last line of the
address, trailing line break characters are also being stripped.
  • Loading branch information
mike3985 committed Dec 18, 2024
1 parent 659de00 commit 210ac6d
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 8 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
replace the whole "\r\n" line break and not just the "\n" character
* Only strip a leading line break from an address block, not just any old first
occurrence of a line break
* Strip trailing whitespace from the lines of an address

## 8.7.0

Expand Down
2 changes: 1 addition & 1 deletion lib/govspeak.rb
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ def render_image(image)
<<~BODY
<div class="address"><div class="adr org fn"><p markdown="1">
#{body.lstrip.gsub(/\\*\r?\n/, '<br />')}
#{body.lstrip.sub(/[\s\\]*\z/, '').gsub(/[ \\]*\r?\n/, '<br />')}
</p></div></div>
BODY
end
Expand Down
28 changes: 21 additions & 7 deletions test/govspeak_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,26 +106,40 @@ class GovspeakTest < Minitest::Test
Teston
0123 456 7890 $A )
doc = Govspeak::Document.new(input)
assert_equal %(\n<div class="address"><div class="adr org fn"><p>\n123 Test Street<br>Testcase Cliffs<br>Teston<br>0123 456 7890 \n</p></div></div>\n), doc.to_html
assert_equal %(\n<div class="address"><div class="adr org fn"><p>\n123 Test Street<br>Testcase Cliffs<br>Teston<br>0123 456 7890\n</p></div></div>\n), doc.to_html
end

test "newlines in address block content are replaced with <br>s" do
input = %($A\n123 Test Street\nTestcase Cliffs\nTeston\n0123 456 7890\n$A)
doc = Govspeak::Document.new(input)
assert_equal %(\n<div class="address"><div class="adr org fn"><p>\n123 Test Street<br>Testcase Cliffs<br>Teston<br>0123 456 7890<br>\n</p></div></div>\n), doc.to_html
assert_equal %(\n<div class="address"><div class="adr org fn"><p>\n123 Test Street<br>Testcase Cliffs<br>Teston<br>0123 456 7890\n</p></div></div>\n), doc.to_html
end

test "combined return-and-newlines in address block content are replaced with <br>s" do
input = %($A\r\n123 Test Street\r\nTestcase Cliffs\r\nTeston\r\n0123 456 7890\r\n$A)
doc = Govspeak::Document.new(input)
assert_equal %(\n<div class="address"><div class="adr org fn"><p>\n123 Test Street<br>Testcase Cliffs<br>Teston<br>0123 456 7890<br>\n</p></div></div>\n), doc.to_html
assert_equal %(\n<div class="address"><div class="adr org fn"><p>\n123 Test Street<br>Testcase Cliffs<br>Teston<br>0123 456 7890\n</p></div></div>\n), doc.to_html
end

test "trailing backslashes are stripped from address block content" do
# two trailing backslashes would normally be converted into a line break by Kramdown
input = %($A\r\n123 Test Street\\\r\nTestcase Cliffs\\\nTeston\\\\\r\n0123 456 7890\\\\\n$A)
doc = Govspeak::Document.new(input)
assert_equal %(\n<div class="address"><div class="adr org fn"><p>\n123 Test Street<br>Testcase Cliffs<br>Teston<br>0123 456 7890<br>\n</p></div></div>\n), doc.to_html
assert_equal %(\n<div class="address"><div class="adr org fn"><p>\n123 Test Street<br>Testcase Cliffs<br>Teston<br>0123 456 7890\n</p></div></div>\n), doc.to_html
end

test "trailing spaces are stripped from address block content" do
# two trailing spaces would normally be converted into a line break by Kramdown
input = %($A\r\n123 Test Street \r\nTestcase Cliffs \nTeston \r\n0123 456 7890 \n$A)
doc = Govspeak::Document.new(input)
assert_equal %(\n<div class="address"><div class="adr org fn"><p>\n123 Test Street<br>Testcase Cliffs<br>Teston<br>0123 456 7890\n</p></div></div>\n), doc.to_html
end

test "trailing backslashes and spaces are stripped from address block content" do
# two trailing backslashes or two trailing spaces would normally be converted into a line break by Kramdown
input = %($A\r\n123 Test Street \\\r\nTestcase Cliffs\\ \nTeston\\\\ \r\n0123 456 7890 \\\\\n$A)
doc = Govspeak::Document.new(input)
assert_equal %(\n<div class="address"><div class="adr org fn"><p>\n123 Test Street<br>Testcase Cliffs<br>Teston<br>0123 456 7890\n</p></div></div>\n), doc.to_html
end

test "should convert barchart" do
Expand Down Expand Up @@ -160,7 +174,7 @@ class GovspeakTest < Minitest::Test
Teston
0123 456 7890 $A)
doc = Govspeak::Document.new(input)
assert_equal %(<p>Paragraph1</p>\n\n<div class="address"><div class="adr org fn"><p>\n123 Test Street<br>Testcase Cliffs<br>Teston<br>0123 456 7890 \n</p></div></div>\n), doc.to_html
assert_equal %(<p>Paragraph1</p>\n\n<div class="address"><div class="adr org fn"><p>\n123 Test Street<br>Testcase Cliffs<br>Teston<br>0123 456 7890\n</p></div></div>\n), doc.to_html
end

test_given_govspeak("^ I am very informational ^") do
Expand Down Expand Up @@ -407,7 +421,7 @@ class GovspeakTest < Minitest::Test
$A" do
assert_html_output %(
<div class="address"><div class="adr org fn"><p>
street<br>road<br>
street<br>road
</p></div></div>)
assert_text_output "street road"
end
Expand All @@ -421,7 +435,7 @@ class GovspeakTest < Minitest::Test
*[ACRONYM]: This is the acronym explanation" do
assert_html_output %(
<div class="address"><div class="adr org fn"><p>
street with <abbr title="This is the acronym explanation">ACRONYM</abbr><br>road<br>
street with <abbr title="This is the acronym explanation">ACRONYM</abbr><br>road
</p></div></div>)
end

Expand Down

0 comments on commit 210ac6d

Please sign in to comment.