vcard: fold parse and lines over one walk
lines had drifted from parse — only the first property of a logical
line holding several content lines survived it — and each re-walked
the same bytes; the error riding the Line keeps the strict raise its
message. Migration 002 reads Line directly and follows it, behavior
unchanged.

Assisted-by: GLM-5.3 via pi
change prnzowqknkumuowxsozvworsmzosquvx
commit 8fe4097c9d46b559646013fbf83c31ed74f9fd42
author Alpha Chen <alpha@kejadlen.dev>
date
parent qquvxlyt
diff --git a/db/migrations/002_birthdays.rb b/db/migrations/002_birthdays.rb
index 8a54ecc..8b62f7b 100644
--- a/db/migrations/002_birthdays.rb
+++ b/db/migrations/002_birthdays.rb
@@ -44,7 +44,7 @@ Sequel.migration do
       # card's BDAY lines take — only the modeled single line moves.
       case ProTacts::VCard.new(row.fetch(:vcard)).lines.partition { it.names?("BDAY") }
       in [[line], others]
-        property = line.property
+        property = line.properties.first
         birthday = property && ProTacts::Birthday.from_property(property)
         next if birthday.nil?
 
diff --git a/lib/pro_tacts/store.rb b/lib/pro_tacts/store.rb
index 9afd0cc..4911c71 100644
--- a/lib/pro_tacts/store.rb
+++ b/lib/pro_tacts/store.rb
@@ -223,7 +223,7 @@ module ProTacts
         case VCard.new(vcard).lines.partition { it.names?("BDAY") }
         in [[line], others]
           report_unrecognized_bday_lines([line])
-          property = line.property
+          property = line.properties.first
           birthday = property && Birthday.from_property(property)
           [birthday, birthday ? others.map(&:verbatim).join : vcard]
         in [[], _]
@@ -390,9 +390,13 @@ module ProTacts
 
       bdays, = VCard.new(vcard).lines.partition { it.names?("BDAY") }
       carried, rest = bdays.partition { |line|
-        line.property && Birthday.unrendered_value?(line.property.value)
+        property = line.properties.first
+        property && Birthday.unrendered_value?(property.value)
+      }
+      lost = rest.reject { |line|
+        property = line.properties.first
+        property && Birthday.rendered?(property)
       }
-      lost = rest.reject { |line| line.property && Birthday.rendered?(line.property) }
       [carried.map(&:verbatim), lost]
     end
 
@@ -405,7 +409,8 @@ module ProTacts
     #: (Array[VCard::Line] lines) -> void
     def report_unrecognized_bday_lines(lines)
       unrecognized = lines.count { |line|
-        line.property.nil? || (!Birthday.rendered?(line.property) && !Birthday.unrendered_value?(line.property.value))
+        property = line.properties.first
+        property.nil? || (!Birthday.rendered?(property) && !Birthday.unrendered_value?(property.value))
       }
       return if unrecognized.zero?
 
diff --git a/lib/pro_tacts/vcard.rb b/lib/pro_tacts/vcard.rb
index 4dbed7a..e7b659a 100644
--- a/lib/pro_tacts/vcard.rb
+++ b/lib/pro_tacts/vcard.rb
@@ -2,14 +2,14 @@ module ProTacts
   # A card, read once: the parsed properties beside the exact bytes
   # they came from, so a caller asks questions of either without
   # re-parsing, and moves a property without a byte around it moving
-  # too. Iteration is explicit over either half — #properties for the
-  # parsed card, #lines for the logical lines parsed beside their
-  # verbatim bytes (Line), where the questions that move bytes are
-  # asked — and #insert puts lines back in. vCard 3.0 (RFC 2426):
-  # escape and fold, the writer's half, are here; Parser owns the
-  # reading half, down to the line split. Both walks are lazy for the
-  # same reason Contact's parse is: the byte-moving paths never read
-  # structure.
+  # too. #lines walks the card once into Line values — a logical line's
+  # properties beside the exact bytes that carried them — and is the
+  # enumeration the questions that move bytes are asked over;
+  # #properties folds the same walk down to structure, and #insert
+  # puts lines back in. vCard 3.0 (RFC 2426): escape and fold, the
+  # writer's half, are here; Parser owns the reading half, down to
+  # the line split. The walk is lazy for the same reason Contact's
+  # parse is: the byte-moving paths never read structure.
   class VCard
     # Folded lines must not exceed 75 octets, excluding the line break
     # (RFC 2426 section 2.6). The octet count, not character count, is
@@ -51,14 +51,16 @@ module ProTacts
     Property = Data.define(:group, :name, :parameters, :value)
 
     # One logical line of a card, parsed beside the exact bytes that
-    # carried it: `property` is the parsed form — nil when the line
-    # will not read as one, which is a fact about the line, not an
-    # error — and `verbatim` is the line itself, folds and terminator
-    # included, so what moves between cards moves unchanged.
+    # carried it: `properties` is every content line the logical line
+    # read (usually one — a lone CR can end one mid-physical-line, and
+    # a blank line reads as none), `error` the ParseError the first
+    # failure raised, nil when it read, and `verbatim` the line itself,
+    # folds and terminator included, so what moves between cards moves
+    # unchanged.
     #
     # The signature lives in sig/pro_tacts/vcard.rbs with Property's.
     # @rbs skip
-    Line = Data.define(:property, :verbatim)
+    Line = Data.define(:properties, :verbatim, :error)
 
     # Reopened rather than defined in the block above: steep reads a
     # define block's self as this class's, not the constant's, so the
@@ -135,16 +137,15 @@ module ProTacts
       @bytes = bytes
     end
 
-    # The card's properties, parsed once on the first structured read.
-    # Nil when the bytes are not a vCard — a caller is expected to carry
-    # on serving the bytes (ParseError's rule, held one level up).
+    # The card's properties, folded off the same single walk #lines is
+    # and parsed once on the first structured read. Nil when any line
+    # would not read — a caller is expected to carry on serving the
+    # bytes (ParseError's rule, held one level up).
     #: () -> Array[Property]?
     def properties
       return @properties if defined?(@properties)
 
-      @properties = Parser.parse(@bytes)
-    rescue ParseError
-      @properties = nil
+      @properties = lines.any? { it.error } ? nil : lines.flat_map { it.properties }
     end
 
     # Whether the parsed properties carry the envelope RFC 2426 section
diff --git a/lib/pro_tacts/vcard/parser.rb b/lib/pro_tacts/vcard/parser.rb
index fe2a95f..28a86e6 100644
--- a/lib/pro_tacts/vcard/parser.rb
+++ b/lib/pro_tacts/vcard/parser.rb
@@ -62,20 +62,25 @@ module ProTacts
       # Every content line in the card, in order. Blank lines are
       # skipped, and BEGIN, VERSION, and END come back like any other
       # property: deciding that a card is well-formed is not this class's
-      # job. Raises on the first line that will not read, so a caller
-      # rescuing ParseError declines the whole card at once.
+      # job. Raises the error of the first line that will not read, so a
+      # caller rescuing ParseError declines the whole card at once.
       #: (String card) -> Array[Property]
       def self.parse(card)
-        logical_lines(card).flat_map { new(it).parse }
+        lines = lines(card)
+        error = lines.filter_map { it.error }.first
+        raise error if error
+
+        lines.flat_map { it.properties }
       end
 
       # The card's logical lines, each parsed beside the exact bytes it
-      # came from, folds and terminator included. A line that will not
-      # read is a fact about the line, not an error: the property is nil
-      # and the bytes stay enumerable.
+      # came from, folds and terminator included — the one walk every
+      # read of a card derives from. A line that will not read is a
+      # fact about the line, not an error: the error it raised rides on
+      # it, and the bytes stay enumerable.
       #: (String card) -> Array[Line]
       def self.lines(card)
-        logical_lines(card).map { |line| Line.new(property: property_of(line), verbatim: line) }
+        logical_lines(card).map { |logical_line| line_of(logical_line) }
       end
 
       # VCard.fold's inverse, and the first thing a read does: RFC 2426
@@ -89,13 +94,15 @@ module ProTacts
         line.gsub(FOLD, "")
       end
 
-      # One logical line read as its first property, or nil when it does
-      # not read as one — blank, or rejected by the grammar.
-      #: (String line) -> Property?
-      def self.property_of(line)
-        new(line).parse.fetch(0, nil)
-      rescue ParseError
-        nil
+      # One logical line into a Line: every content line it read, or
+      # the error the first failure raised — the prefix that did read
+      # is dropped to match parse, which declines a line wholesale by
+      # raising. A blank line is neither: no properties, no error.
+      #: (String logical_line) -> Line
+      def self.line_of(logical_line)
+        Line.new(properties: new(logical_line).parse, verbatim: logical_line, error: nil)
+      rescue ParseError => error
+        Line.new(properties: [], verbatim: logical_line, error:)
       end
 
       # The card's logical lines: a physical line and the continuations
@@ -120,7 +127,7 @@ module ProTacts
         card.split(/(?<=\n)/, -1)
       end
 
-      private_class_method :property_of, :logical_lines, :physical_lines
+      private_class_method :line_of, :logical_lines, :physical_lines
 
       #: (String logical_line) -> void
       def initialize(logical_line)
diff --git a/sig/pro_tacts/vcard.rbs b/sig/pro_tacts/vcard.rbs
index e94d8a5..5be6970 100644
--- a/sig/pro_tacts/vcard.rbs
+++ b/sig/pro_tacts/vcard.rbs
@@ -13,10 +13,11 @@ module ProTacts
     end
 
     class Line
-      attr_reader property: Property?
+      attr_reader properties: Array[Property]
       attr_reader verbatim: String
+      attr_reader error: ParseError?
 
-      def self.new: (property: Property?, verbatim: String) -> instance
+      def self.new: (properties: Array[Property], verbatim: String, error: ParseError?) -> instance
     end
   end
 end
diff --git a/test/pro_tacts/vcard/test_lines.rb b/test/pro_tacts/vcard/test_lines.rb
index a0ee385..808e179 100644
--- a/test/pro_tacts/vcard/test_lines.rb
+++ b/test/pro_tacts/vcard/test_lines.rb
@@ -54,7 +54,7 @@ class VCardLinesTest < Minitest::Test
   def test_lines_yield_the_cards_lines_parsed_beside_verbatim
     lines = VCARD.new(CARD).lines
 
-    assert_equal %w[BEGIN VERSION FN UID END], lines.map { it.property&.name }
+    assert_equal %w[BEGIN VERSION FN UID END], lines.map { it.properties.first&.name }
     assert_equal CARD, lines.map(&:verbatim).join
   end
 
@@ -63,7 +63,7 @@ class VCardLinesTest < Minitest::Test
     bdays, others = VCARD.new(born).lines.partition { it.names?("BDAY") }
 
     assert_equal 1, bdays.length
-    assert_equal "1985-04-12", bdays.fetch(0).property&.value
+    assert_equal "1985-04-12", bdays.fetch(0).properties.first&.value
     assert_equal "BDAY:1985-04-12\r\n", bdays.fetch(0).verbatim
     assert_equal CARD, others.map(&:verbatim).join
   end
@@ -74,13 +74,13 @@ class VCardLinesTest < Minitest::Test
     folded = CARD.sub("UID:ada\r\n", "UID:ada\r\nBDAY:1985-04-\r\n 12\r\n")
     bday = VCARD.new(folded).lines.find { it.names?("BDAY") }
 
-    assert_equal "1985-04-12", bday&.property&.value
+    assert_equal "1985-04-12", bday&.properties&.first&.value
     assert_equal "BDAY:1985-04-\r\n 12\r\n", bday&.verbatim
   end
 
   # A group prefix counts as naming the property; anything else does
-  # not, and a line that will not parse is a fact about the line
-  # (property nil), not an error.
+  # not, and a line that will not parse is a fact about the line,
+  # carried as data rather than raised.
   def test_names_judges_lines_not_properties
     grouped = CARD.sub("UID:ada\r\n", "UID:ada\r\nitem1.BDAY:1985-04-12\r\n")
 
@@ -92,7 +92,8 @@ class VCardLinesTest < Minitest::Test
 
     unparsed = VCARD.new("BDAY;=;:\r\n").lines.first
 
-    assert_nil unparsed.property
+    assert_kind_of VCARD::ParseError, unparsed.error
+    assert_empty unparsed.properties
     assert unparsed.names?("BDAY")
     assert_equal "BDAY;=;:\r\n", unparsed.verbatim
   end
diff --git a/test/pro_tacts/vcard/test_parser.rb b/test/pro_tacts/vcard/test_parser.rb
index 233a860..4e1e039 100644
--- a/test/pro_tacts/vcard/test_parser.rb
+++ b/test/pro_tacts/vcard/test_parser.rb
@@ -153,26 +153,42 @@ class VCardParserTest < Minitest::Test
     card = "BEGIN:VCARD\r\nBDAY:1985-04-\r\n 12\r\nEND:VCARD\r\n"
     lines = ProTacts::VCard::Parser.lines(card)
 
-    assert_equal %w[BEGIN BDAY END], lines.map { it.property&.name }
-    assert_equal "1985-04-12", lines.fetch(1).property&.value
+    assert_equal %w[BEGIN BDAY END], lines.map { it.properties.first&.name }
+    assert_equal "1985-04-12", lines.fetch(1).properties.first&.value
     assert_equal "BDAY:1985-04-\r\n 12\r\n", lines.fetch(1).verbatim
     assert_equal card, lines.map(&:verbatim).join
   end
 
-  def test_a_line_that_will_not_read_has_no_property_but_keeps_its_bytes
+  # A lone CR ends a content line inside a physical line, so one
+  # logical line can read as two properties — the walk keeps both
+  # beside the bytes that carried them, where the old lenient read
+  # kept only the first.
+  def test_a_logical_line_can_read_as_several_content_lines
+    line = ProTacts::VCard::Parser.lines("FN:A\rNOTE:n\r\n").fetch(0)
+
+    assert_equal %w[FN NOTE], line.properties.map { it.name }
+    assert_nil line.error
+    assert_equal %w[FN NOTE], parse("FN:A\rNOTE:n\r\n").map { it.name }
+  end
+
+  def test_a_line_that_will_not_read_carries_its_error_and_keeps_its_bytes
     line = ProTacts::VCard::Parser.lines("BDAY;=;:\r\n").fetch(0)
 
-    assert_nil line.property
+    assert_kind_of ProTacts::VCard::ParseError, line.error
+    assert_empty line.properties
     assert_equal "BDAY;=;:\r\n", line.verbatim
   end
 
   # The two reads' strictness split, pinned: parse declines the whole
-  # card, lines carries the same line with a nil property.
+  # card, lines carries the same line with its error — and the raise is
+  # that error, not a fresh one.
   def test_parse_raises_where_lines_carries
     card = "FN:A\r\nBDAY;=;:\r\n"
+    carried = ProTacts::VCard::Parser.lines(card).fetch(1)
+
+    error = assert_raises(ProTacts::VCard::ParseError) { parse(card) }
 
-    assert_raises(ProTacts::VCard::ParseError) { parse(card) }
-    assert_nil ProTacts::VCard::Parser.lines(card).fetch(1).property
+    assert_equal carried.error.message, error.message
   end
 
   # A blank line is a line — its bytes stay enumerable, so a rewrite
@@ -182,7 +198,8 @@ class VCardParserTest < Minitest::Test
     lines = ProTacts::VCard::Parser.lines("FN:A\r\n\r\nUID:b\r\n")
 
     assert_equal ["FN:A\r\n", "\r\n", "UID:b\r\n"], lines.map(&:verbatim)
-    assert_nil lines.fetch(1).property
+    assert_empty lines.fetch(1).properties
+    assert_nil lines.fetch(1).error
     assert_equal %w[FN UID], parse("FN:A\r\n\r\nUID:b\r\n").map { it.name }
   end