From 6c6abded69e1cd4d13157f3d5924b7686e140224 Mon Sep 17 00:00:00 2001 From: Romaric Date: Mon, 21 Nov 2022 14:16:50 +0000 Subject: [PATCH 1/5] Add examples of headings shared by pages with wrong IDs --- example/source/shared-subheadings/page-a.html.md | 14 ++++++++++++++ example/source/shared-subheadings/page-b.html.md | 14 ++++++++++++++ 2 files changed, 28 insertions(+) create mode 100644 example/source/shared-subheadings/page-a.html.md create mode 100644 example/source/shared-subheadings/page-b.html.md diff --git a/example/source/shared-subheadings/page-a.html.md b/example/source/shared-subheadings/page-a.html.md new file mode 100644 index 00000000..037eb3be --- /dev/null +++ b/example/source/shared-subheadings/page-a.html.md @@ -0,0 +1,14 @@ +--- +layout: layout +--- + +# Shared subheadings - Page A + +A page that shares headings with the other pages in this folder, +to check that the heading IDs are computed properly + +## A subheading + +### A h3 subheading + +## Another subheading diff --git a/example/source/shared-subheadings/page-b.html.md b/example/source/shared-subheadings/page-b.html.md new file mode 100644 index 00000000..1acba227 --- /dev/null +++ b/example/source/shared-subheadings/page-b.html.md @@ -0,0 +1,14 @@ +--- +layout: layout +--- + +# Shared subheadings - Page B + +A page that shares headings with the other pages in this folder, +to check that the heading IDs are computed properly + +## A subheading + +### A h3 subheading + +## Another subheading From 298bf7d3b9d7bada94ed1192a327f1c44ca3cd57 Mon Sep 17 00:00:00 2001 From: Romaric Date: Mon, 21 Nov 2022 14:37:48 +0000 Subject: [PATCH 2/5] Add test for generation of heading IDs while rendering markdown --- .../tech_docs_html_renderer.rb | 1 + .../tech_docs_html_renderer_spec.rb | 36 +++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/lib/govuk_tech_docs/tech_docs_html_renderer.rb b/lib/govuk_tech_docs/tech_docs_html_renderer.rb index f9f2d614..241d8dbf 100644 --- a/lib/govuk_tech_docs/tech_docs_html_renderer.rb +++ b/lib/govuk_tech_docs/tech_docs_html_renderer.rb @@ -1,4 +1,5 @@ require "middleman-core/renderers/redcarpet" +require "govuk_tech_docs/unique_identifier_generator" module GovukTechDocs class TechDocsHTMLRenderer < Middleman::Renderers::MiddlemanRedcarpetHTML diff --git a/spec/govuk_tech_docs/tech_docs_html_renderer_spec.rb b/spec/govuk_tech_docs/tech_docs_html_renderer_spec.rb index 83723aad..deadf61a 100644 --- a/spec/govuk_tech_docs/tech_docs_html_renderer_spec.rb +++ b/spec/govuk_tech_docs/tech_docs_html_renderer_spec.rb @@ -95,4 +95,40 @@ def hello_world end end end + + describe "#render unique heading IDs" do + # Reset the UniqueIdentifierGenerator between each tests to ensure independence + before(:each) { GovukTechDocs::UniqueIdentifierGenerator.instance.reset } + + it "Automatically assigns an ID to the heading" do + output = processor.render <<~MARKDOWN + # A heading + ## A subheading + MARKDOWN + + expect(output).to include('

A heading

') + expect(output).to include('

A subheading

') + end + + it "Ensures IDs are unique among headings in the page" do + output = processor.render <<~MARKDOWN + # A heading + ## A subheading + ### A shared heading + ### A shared heading + ### A shared heading + ## Another subheading + ### A shared heading + MARKDOWN + + # Fist heading will get its ID as a normal heading + expect(output).to include('

A shared heading

') + # Second occurence will be prefixed by the previous heading to ensure uniqueness + expect(output).to include('

A shared heading

') + # Third occurence will be get the prefix, as well as a numeric suffix, to keep ensuring uniqueness + expect(output).to include('

A shared heading

') + # Finally the last occurence will get a prefix, but no number as it's in a different section + expect(output).to include('

A shared heading

') + end + end end From d9beef0460fc767f026ca940eadc33950ba78fd9 Mon Sep 17 00:00:00 2001 From: Romaric Date: Mon, 21 Nov 2022 14:40:31 +0000 Subject: [PATCH 3/5] Reset UniqueIdentifierGenerator between each render Prevents IDs to be misinterpreted as duplicated across the rendering of different pages sharing headings --- lib/govuk_tech_docs/tech_docs_html_renderer.rb | 6 ++++++ .../tech_docs_html_renderer_spec.rb | 16 +++++++++++++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/lib/govuk_tech_docs/tech_docs_html_renderer.rb b/lib/govuk_tech_docs/tech_docs_html_renderer.rb index 241d8dbf..35fa653a 100644 --- a/lib/govuk_tech_docs/tech_docs_html_renderer.rb +++ b/lib/govuk_tech_docs/tech_docs_html_renderer.rb @@ -11,6 +11,12 @@ def initialize(options = {}) super end + def preprocess(document) + UniqueIdentifierGenerator.instance.reset + + document + end + def paragraph(text) @app.api("

#{text.strip}

\n") end diff --git a/spec/govuk_tech_docs/tech_docs_html_renderer_spec.rb b/spec/govuk_tech_docs/tech_docs_html_renderer_spec.rb index deadf61a..5ed75d39 100644 --- a/spec/govuk_tech_docs/tech_docs_html_renderer_spec.rb +++ b/spec/govuk_tech_docs/tech_docs_html_renderer_spec.rb @@ -109,7 +109,7 @@ def hello_world expect(output).to include('

A heading

') expect(output).to include('

A subheading

') end - + it "Ensures IDs are unique among headings in the page" do output = processor.render <<~MARKDOWN # A heading @@ -130,5 +130,19 @@ def hello_world # Finally the last occurence will get a prefix, but no number as it's in a different section expect(output).to include('

A shared heading

') end + + it "Does not consider unique IDs across multiple renders" do + processor.render <<~MARKDOWN + # A heading + ## A subheading + MARKDOWN + + second_output = processor.render <<~MARKDOWN + # A heading + ## A subheading + MARKDOWN + + expect(second_output).to include('

A subheading

') + end end end From 0a894e5b0190339f3622b35aac93f0ee6f4cc74b Mon Sep 17 00:00:00 2001 From: Romaric Date: Mon, 21 Nov 2022 16:02:58 +0000 Subject: [PATCH 4/5] Remove UniqueIdentifierExtension for Middleman It is no longer needed for resetting the generator on the before lifecycle hook as it gets reset by the Markdown renderer --- lib/govuk_tech_docs.rb | 2 -- lib/govuk_tech_docs/unique_identifier_extension.rb | 13 ------------- 2 files changed, 15 deletions(-) delete mode 100644 lib/govuk_tech_docs/unique_identifier_extension.rb diff --git a/lib/govuk_tech_docs.rb b/lib/govuk_tech_docs.rb index 4cacd978..37843991 100644 --- a/lib/govuk_tech_docs.rb +++ b/lib/govuk_tech_docs.rb @@ -18,7 +18,6 @@ require "govuk_tech_docs/page_review" require "govuk_tech_docs/pages" require "govuk_tech_docs/tech_docs_html_renderer" -require "govuk_tech_docs/unique_identifier_extension" require "govuk_tech_docs/unique_identifier_generator" require "govuk_tech_docs/warning_text_extension" require "govuk_tech_docs/api_reference/api_reference_extension" @@ -62,7 +61,6 @@ def self.configure(context, options = {}) config_file = ENV.fetch("CONFIG_FILE", "config/tech-docs.yml") context.config[:tech_docs] = YAML.load_file(config_file).with_indifferent_access - context.activate :unique_identifier context.activate :warning_text context.activate :api_reference diff --git a/lib/govuk_tech_docs/unique_identifier_extension.rb b/lib/govuk_tech_docs/unique_identifier_extension.rb deleted file mode 100644 index bb50e943..00000000 --- a/lib/govuk_tech_docs/unique_identifier_extension.rb +++ /dev/null @@ -1,13 +0,0 @@ -module GovukTechDocs - class UniqueIdentifierExtension < Middleman::Extension - def initialize(app, options_hash = {}, &block) - super - - app.before do - UniqueIdentifierGenerator.instance.reset - end - end - end -end - -::Middleman::Extensions.register(:unique_identifier, GovukTechDocs::UniqueIdentifierExtension) From 38ad27857f875abcc4204efde53fadc165c53a00 Mon Sep 17 00:00:00 2001 From: Romaric Date: Mon, 21 Nov 2022 16:10:10 +0000 Subject: [PATCH 5/5] Stop UniqueIdentifierGenerator from being a Singleton It no longer needs to be as its lifecycle is now fully tied to the Redcarpet renderer --- lib/govuk_tech_docs/tech_docs_html_renderer.rb | 4 ++-- lib/govuk_tech_docs/unique_identifier_generator.rb | 10 +--------- spec/govuk_tech_docs/tech_docs_html_renderer_spec.rb | 3 --- .../unique_identifier_generator_spec.rb | 12 +----------- 4 files changed, 4 insertions(+), 25 deletions(-) diff --git a/lib/govuk_tech_docs/tech_docs_html_renderer.rb b/lib/govuk_tech_docs/tech_docs_html_renderer.rb index 35fa653a..3eef19e3 100644 --- a/lib/govuk_tech_docs/tech_docs_html_renderer.rb +++ b/lib/govuk_tech_docs/tech_docs_html_renderer.rb @@ -12,7 +12,7 @@ def initialize(options = {}) end def preprocess(document) - UniqueIdentifierGenerator.instance.reset + @unique_identifier_generator = UniqueIdentifierGenerator.new document end @@ -22,7 +22,7 @@ def paragraph(text) end def header(text, level) - anchor = UniqueIdentifierGenerator.instance.create(text, level) + anchor = @unique_identifier_generator.create(text, level) %(#{text}\n) end diff --git a/lib/govuk_tech_docs/unique_identifier_generator.rb b/lib/govuk_tech_docs/unique_identifier_generator.rb index 62b932c5..6006fcb7 100644 --- a/lib/govuk_tech_docs/unique_identifier_generator.rb +++ b/lib/govuk_tech_docs/unique_identifier_generator.rb @@ -1,15 +1,11 @@ -require "singleton" - module GovukTechDocs class UniqueIdentifierGenerator - include Singleton - Anchor = Struct.new(:id, :level) attr_reader :anchors def initialize - reset + @anchors = [] end def create(id, level) @@ -28,10 +24,6 @@ def create(id, level) anchor end - def reset - @anchors = [] - end - private def prefixed_by_parent(anchor, level) diff --git a/spec/govuk_tech_docs/tech_docs_html_renderer_spec.rb b/spec/govuk_tech_docs/tech_docs_html_renderer_spec.rb index 5ed75d39..fcba3f8f 100644 --- a/spec/govuk_tech_docs/tech_docs_html_renderer_spec.rb +++ b/spec/govuk_tech_docs/tech_docs_html_renderer_spec.rb @@ -97,9 +97,6 @@ def hello_world end describe "#render unique heading IDs" do - # Reset the UniqueIdentifierGenerator between each tests to ensure independence - before(:each) { GovukTechDocs::UniqueIdentifierGenerator.instance.reset } - it "Automatically assigns an ID to the heading" do output = processor.render <<~MARKDOWN # A heading diff --git a/spec/govuk_tech_docs/unique_identifier_generator_spec.rb b/spec/govuk_tech_docs/unique_identifier_generator_spec.rb index 074ff1a0..6c79f851 100644 --- a/spec/govuk_tech_docs/unique_identifier_generator_spec.rb +++ b/spec/govuk_tech_docs/unique_identifier_generator_spec.rb @@ -1,6 +1,5 @@ RSpec.describe GovukTechDocs::UniqueIdentifierGenerator do - subject { described_class.instance } - before { subject.reset } + subject { described_class.new } describe "#create" do it "lower-cases the text" do @@ -66,13 +65,4 @@ end end end - - describe "#reset" do - it "clears the list of existing anchors" do - subject.create("An Anchor", 1) - expect(subject.anchors).to_not be_empty - subject.reset - expect(subject.anchors).to be_empty - end - end end