Harden series tags: italic markdown, empty-literal errors, cleanup

series_link now passes italic: true to MarkdownLinkFormatter so series
names render as _italic_ in markdown mode. series_text switches from
hand-rolled markdown links to MarkdownLinkFormatter with self-link
suppression via LinkHelperUtils.

Both tags now reject empty quoted literals ('', "", '   ') at parse
time instead of silently rendering empty strings. Variables that
resolve to empty at runtime are still handled by the render-time path.

Changes:
- Add italic: true to series_link markdown render path.
- Refactor series_text render_markdown to use MdLink and LinkHelper.
- Add validate_title/validate_series_name checks for empty quoted
  literals via regex matching.
- Remove unused require 'cgi' and stale comments from both tags.
- Add tests: link_text override in markdown, self-link with "The"
  prefix, bracket escaping, empty literal parse errors (6 new).
- Update 4 existing tests from render-time empty checks to parse-time
  SyntaxError assertions.

Author: agude

_plugins/src/content/series/tags/series_link_tag.rb

@@ -3,7 +3,6 @@
 # _plugins/series_link_tag.rb
 require 'jekyll'
 require 'liquid'
-require 'cgi' # Keep for QuotedFragment, though CGI itself is now in LiquidUtils
 require 'strscan'
 require_relative '../series_link_util'
 require_relative '../../../infrastructure/tag_argument_utils'
@@ -49,7 +48,7 @@ def render(context)
 
           if context.registers[:render_mode] == :markdown
             data = Linker.find_series_link_data(series_title, context, link_text_override)
-            MdLink.format_link(data, self_link: LinkHelper.self_link?(context, data[:url]))
+            MdLink.format_link(data, italic: true, self_link: LinkHelper.self_link?(context, data[:url]))
           else
             Linker.render_series_link(series_title, context, link_text_override)
           end
@@ -95,8 +94,15 @@ def parse_link_text_argument(scanner)
         end
 
         def validate_title
-          return if @title_markup && !@title_markup.strip.empty?
+          raise_empty_title if @title_markup.nil? || @title_markup.strip.empty?
 
+          m = @title_markup.match(/\A(['"])(.*)\1\z/m)
+          return unless m
+
+          raise_empty_title if m[2].strip.empty?
+        end
+
+        def raise_empty_title
           raise Liquid::SyntaxError,
                 "Syntax Error in 'series_link': " \
                 "Title value is missing or empty in '#{@raw_markup}'"

_plugins/src/content/series/tags/series_text_tag.rb

@@ -3,12 +3,12 @@
 # _plugins/series_text_tag.rb
 require 'jekyll'
 require 'liquid'
-# CGI and strscan are still used by the tag's initialize
-require 'cgi'
 require 'strscan'
 require_relative '../series_link_util'
 require_relative '../../../infrastructure/tag_argument_utils'
-require_relative '../series_text_utils' # Require the new utility
+require_relative '../series_text_utils'
+require_relative '../../markdown_output/markdown_link_formatter'
+require_relative '../../../infrastructure/links/link_helper_utils'
 
 # Renders series text with appropriate context and linking.
 #
@@ -30,7 +30,9 @@ class SeriesTextTag < Liquid::Tag
         TagArgs = Jekyll::Infrastructure::TagArgumentUtils
         Linker = Jekyll::Series::SeriesLinkUtils
         TextUtil = Jekyll::Series::SeriesTextUtils
-        private_constant :TagArgs, :Linker, :TextUtil
+        MdLink = Jekyll::MarkdownOutput::MarkdownLinkFormatter
+        LinkHelper = Jekyll::Infrastructure::Links::LinkHelperUtils
+        private_constant :TagArgs, :Linker, :TextUtil, :MdLink, :LinkHelper
 
         QuotedFragment = Liquid::QuotedFragment
 
@@ -108,20 +110,24 @@ def validate_no_extra_arguments(scanner)
         end
 
         def validate_series_name
-          return if @series_name_markup && !@series_name_markup.strip.empty?
+          raise_empty_name if @series_name_markup.nil? || @series_name_markup.strip.empty?
 
+          m = @series_name_markup.match(/\A(['"])(.*)\1\z/m)
+          return unless m
+
+          raise_empty_name if m[2].strip.empty?
+        end
+
+        def raise_empty_name
           raise Liquid::SyntaxError,
                 "Syntax Error in 'series_text': " \
                 "Series name value is missing or empty in '#{@raw_markup}'"
         end
 
         def render_markdown(analysis, context, link)
           data = Linker.find_series_link_data(analysis[:name], context, nil, link: link)
-          text = if data[:url]
-                   "[#{data[:display_text]}](#{data[:url]})"
-                 else
-                   data[:display_text]
-                 end
+          self_link = LinkHelper.self_link?(context, data[:url])
+          text = MdLink.format_link(data, italic: true, self_link: self_link)
           "#{analysis[:prefix]}#{text}#{analysis[:suffix]}".strip
         end
 

_tests/src/content/series/tags/test_series_link_tag.rb

@@ -43,13 +43,26 @@ def test_syntax_error_missing_series_title
     assert_match 'Could not find series title', err.message
   end
 
-  # Commenting out the problematic test for empty string literal, assuming similar issue
-  # def test_syntax_error_series_title_empty_string_literal
-  #   err = assert_raises Liquid::SyntaxError do
-  #     Liquid::Template.parse("{% series_link '' %}")
-  #   end
-  #   assert_match "Title value is missing or empty", err.message
-  # end
+  def test_syntax_error_empty_single_quoted_title
+    err = assert_raises Liquid::SyntaxError do
+      Liquid::Template.parse("{% series_link '' %}")
+    end
+    assert_match 'Title value is missing or empty', err.message
+  end
+
+  def test_syntax_error_empty_double_quoted_title
+    err = assert_raises Liquid::SyntaxError do
+      Liquid::Template.parse('{% series_link "" %}')
+    end
+    assert_match 'Title value is missing or empty', err.message
+  end
+
+  def test_syntax_error_whitespace_only_quoted_title
+    err = assert_raises Liquid::SyntaxError do
+      Liquid::Template.parse("{% series_link '   ' %}")
+    end
+    assert_match 'Title value is missing or empty', err.message
+  end
 
   def test_syntax_error_unknown_argument
     err = assert_raises Liquid::SyntaxError do
@@ -137,7 +150,7 @@ def test_markdown_mode_renders_markdown_link
     )
     template = Liquid::Template.parse("{% series_link 'Hyperion Cantos' %}")
     output = template.render!(md_context)
-    assert_equal '[Hyperion Cantos](/books/series/hyperion-cantos/)', output
+    assert_equal '[_Hyperion Cantos_](/books/series/hyperion-cantos/)', output
   end
 
   def test_markdown_mode_not_found_renders_plain_text
@@ -148,6 +161,51 @@ def test_markdown_mode_not_found_renders_plain_text
     )
     template = Liquid::Template.parse("{% series_link 'Unknown Series' %}")
     output = template.render!(md_context)
-    assert_equal 'Unknown Series', output
+    assert_equal '_Unknown Series_', output
+  end
+
+  def test_markdown_mode_self_link_renders_italic_text
+    series_page = create_doc(
+      { 'title' => 'Hyperion Cantos', 'layout' => 'series_page' },
+      '/books/series/hyperion-cantos/',
+    )
+    site = create_site({}, {}, [series_page])
+    md_context = create_context(
+      {},
+      { site: site, page: series_page, render_mode: :markdown },
+    )
+    template = Liquid::Template.parse("{% series_link 'Hyperion Cantos' %}")
+    output = template.render!(md_context)
+    assert_equal '_Hyperion Cantos_', output
+  end
+
+  def test_markdown_mode_with_link_text_override
+    series_page = create_doc(
+      { 'title' => 'Hyperion Cantos', 'layout' => 'series_page' },
+      '/books/series/hyperion-cantos/',
+    )
+    site = create_site({}, {}, [series_page])
+    md_context = create_context(
+      {},
+      { site: site, page: create_doc({}, '/test.html'), render_mode: :markdown },
+    )
+    template = Liquid::Template.parse("{% series_link 'Hyperion Cantos' link_text='the series' %}")
+    output = template.render!(md_context)
+    assert_equal '[_the series_](/books/series/hyperion-cantos/)', output
+  end
+
+  def test_markdown_mode_escapes_brackets_in_title
+    series_page = create_doc(
+      { 'title' => 'Foundation [Novels]', 'layout' => 'series_page' },
+      '/books/series/foundation-novels/',
+    )
+    site = create_site({}, {}, [series_page])
+    md_context = create_context(
+      {},
+      { site: site, page: create_doc({}, '/test.html'), render_mode: :markdown },
+    )
+    template = Liquid::Template.parse("{% series_link 'Foundation [Novels]' %}")
+    output = template.render!(md_context)
+    assert_equal '[_Foundation \[Novels\]_](/books/series/foundation-novels/)', output
   end
 end

_tests/src/content/series/tags/test_series_text_tag.rb

@@ -154,8 +154,10 @@ def test_series_with_variable
   end
 
   def test_empty_input_string
-    markup = '""'
-    assert_equal '', render_tag(markup)
+    err = assert_raises Liquid::SyntaxError do
+      Liquid::Template.parse('{% series_text "" %}')
+    end
+    assert_match(/Series name value is missing or empty/, err.message)
   end
 
   def test_empty_input_variable
@@ -171,8 +173,10 @@ def test_nil_input_variable
   end
 
   def test_whitespace_input_string
-    markup = '"   "'
-    assert_equal '', render_tag(markup)
+    err = assert_raises Liquid::SyntaxError do
+      Liquid::Template.parse('{% series_text "   " %}')
+    end
+    assert_match(/Series name value is missing or empty/, err.message)
   end
 
   def test_keyword_as_substring_does_not_prevent_suffix
@@ -189,6 +193,27 @@ def test_syntax_error_missing_argument
     assert_match(/Could not find series name/, err.message)
   end
 
+  def test_syntax_error_empty_single_quoted_name
+    err = assert_raises Liquid::SyntaxError do
+      Liquid::Template.parse("{% series_text '' %}")
+    end
+    assert_match(/Series name value is missing or empty/, err.message)
+  end
+
+  def test_syntax_error_empty_double_quoted_name
+    err = assert_raises Liquid::SyntaxError do
+      Liquid::Template.parse('{% series_text "" %}')
+    end
+    assert_match(/Series name value is missing or empty/, err.message)
+  end
+
+  def test_syntax_error_whitespace_only_quoted_name
+    err = assert_raises Liquid::SyntaxError do
+      Liquid::Template.parse("{% series_text '   ' %}")
+    end
+    assert_match(/Series name value is missing or empty/, err.message)
+  end
+
   def test_syntax_error_extra_argument
     err = assert_raises Liquid::SyntaxError do
       Liquid::Template.parse('{% series_text "Foundation" extra=arg %}')
@@ -209,31 +234,51 @@ def test_series_ending_with_series_word
   def test_markdown_mode_renders_linked_series
     md_context = create_context({}, { site: @site, page: @current_page, render_mode: :markdown })
     output = Liquid::Template.parse('{% series_text "Foundation" %}').render!(md_context)
-    assert_equal 'the [Foundation](/series/foundation.html) series', output
+    assert_equal 'the [_Foundation_](/series/foundation.html) series', output
   end
 
   def test_markdown_mode_renders_unlinked_series
     md_context = create_context({}, { site: @site, page: @current_page, render_mode: :markdown })
     output = Liquid::Template.parse('{% series_text "Unknown" %}').render!(md_context)
-    assert_equal 'the Unknown series', output
+    assert_equal 'the _Unknown_ series', output
   end
 
   def test_markdown_mode_series_starting_with_the
     md_context = create_context({}, { site: @site, page: @current_page, render_mode: :markdown })
     output = Liquid::Template.parse('{% series_text "The Expanse" %}').render!(md_context)
-    assert_equal '[The Expanse](/series/expanse.html) series', output
+    assert_equal '[_The Expanse_](/series/expanse.html) series', output
   end
 
   def test_markdown_mode_series_containing_keyword
     md_context = create_context({}, { site: @site, page: @current_page, render_mode: :markdown })
     output = Liquid::Template.parse('{% series_text "Dune Saga" %}').render!(md_context)
-    assert_equal 'the [Dune Saga](/series/dune.html)', output
+    assert_equal 'the [_Dune Saga_](/series/dune.html)', output
   end
 
   def test_markdown_mode_empty_input
-    md_context = create_context({}, { site: @site, page: @current_page, render_mode: :markdown })
-    output = Liquid::Template.parse('{% series_text "" %}').render!(md_context)
-    assert_equal '', output
+    err = assert_raises Liquid::SyntaxError do
+      Liquid::Template.parse('{% series_text "" %}')
+    end
+    assert_match(/Series name value is missing or empty/, err.message)
+  end
+
+  def test_markdown_mode_self_link_suppressed
+    # When rendering a series page's own markdown, the link should be suppressed
+    md_context = create_context(
+      {},
+      { site: @site, page: @series1_page, render_mode: :markdown },
+    )
+    output = Liquid::Template.parse('{% series_text "Foundation" %}').render!(md_context)
+    assert_equal 'the _Foundation_ series', output
+  end
+
+  def test_markdown_mode_self_link_with_the_prefix
+    md_context = create_context(
+      {},
+      { site: @site, page: @series2_page, render_mode: :markdown },
+    )
+    output = Liquid::Template.parse('{% series_text "The Expanse" %}').render!(md_context)
+    assert_equal '_The Expanse_ series', output
   end
 
   # --- link=false ---
@@ -271,13 +316,13 @@ def test_link_true_explicit_still_links
   def test_link_false_markdown_mode_renders_plain_text
     md_context = create_context({}, { site: @site, page: @current_page, render_mode: :markdown })
     output = Liquid::Template.parse('{% series_text "Foundation" link=false %}').render!(md_context)
-    assert_equal 'the Foundation series', output
+    assert_equal 'the _Foundation_ series', output
   end
 
   def test_link_false_markdown_mode_series_starting_with_the
     md_context = create_context({}, { site: @site, page: @current_page, render_mode: :markdown })
     output = Liquid::Template.parse('{% series_text "The Expanse" link=false %}').render!(md_context)
-    assert_equal 'The Expanse series', output
+    assert_equal '_The Expanse_ series', output
   end
 
   def test_link_false_with_variable
@@ -294,9 +339,11 @@ def test_syntax_error_unknown_argument_still_raises
     assert_match(/Unexpected arguments/, err.message)
   end
 
-  def test_link_false_empty_string_returns_empty
-    output = render_tag('"" link=false')
-    assert_equal '', output
+  def test_link_false_empty_string_raises
+    err = assert_raises Liquid::SyntaxError do
+      Liquid::Template.parse('{% series_text "" link=false %}')
+    end
+    assert_match(/Series name value is missing or empty/, err.message)
   end
 
   def test_link_false_variable_resolves_to_unknown_series