diff --git a/PR.md b/PR.md new file mode 100644 index 0000000..009e48f --- /dev/null +++ b/PR.md @@ -0,0 +1,73 @@ +# fix: honor start="0" on ordered lists + +## Problem + +Converting an ordered list whose `start` attribute is `0` produces the wrong +numbering: the list restarts at `1` instead of `0`. + +```php +$converter = new League\HTMLToMarkdown\HtmlConverter(); +$converter->convert('
  1. Item A
  2. Item B
'); +// actual: "1. Item A\n2. Item B" +// expected: "0. Item A\n1. Item B" +``` + +`
    ` is valid HTML, and CommonMark supports ordered lists that begin +at `0`, so the resulting Markdown is incorrect. Any other start value (for +example `start="120"`) works. + +## Root cause + +`src/Converter/ListItemConverter.php:55`. The start value was read inside a +truthiness check: + +```php +if ($listType === 'ol' && ($parent = $element->getParent()) && ($start = \intval($parent->getAttribute('start')))) { + $number = $start + $element->getSiblingPosition() - 1; +} else { + $number = $element->getSiblingPosition(); +} +``` + +`\intval('0')` is `0`, which is falsy, so an explicit `start="0"` takes the +`else` branch and the list is renumbered from the sibling position (1-based). +The check cannot distinguish "start attribute absent" from "start attribute set +to 0", because `getAttribute('start')` returns `''` when the attribute is missing +and `intval('')` is also `0`. + +## The fix + +Detect an explicitly provided numeric `start` attribute with `\is_numeric()` +instead of relying on the truthiness of `intval()`: + +```php +if ($listType === 'ol' && ($parent = $element->getParent()) && \is_numeric($start = $parent->getAttribute('start'))) { + $number = (int) $start + $element->getSiblingPosition() - 1; +} else { + $number = $element->getSiblingPosition(); +} +``` + +`\is_numeric('')` is `false` (missing attribute -> normal numbering) while +`\is_numeric('0')` is `true`, so `start="0"` is now respected. Existing values +such as `start="120"` are unaffected. + +## Test + +Added an assertion in `HtmlConverterTest::testLists`: + +```php +$this->assertHtmlGivesMarkdown('
    1. Item A
    2. Item B
    ', "0. Item A\n1. Item B"); +``` + +## Gate + +``` +composer install && vendor/bin/phpunit --no-coverage +``` + +Result: exit code `0` - `OK (50 tests, 230 assertions)`. + +Counterfactual: reverting `ListItemConverter` to the `intval()` truthiness check +fails the new assertion - `
      ` yields `1. Item A\n2. Item B` instead +of `0. Item A\n1. Item B`. diff --git a/src/Converter/ListItemConverter.php b/src/Converter/ListItemConverter.php index d71f601..a61b971 100644 --- a/src/Converter/ListItemConverter.php +++ b/src/Converter/ListItemConverter.php @@ -52,8 +52,8 @@ class ListItemConverter implements ConverterInterface, ConfigurationAwareInterfa return $prefix . $this->listItemStyle . ' ' . $value . "\n"; } - if ($listType === 'ol' && ($parent = $element->getParent()) && ($start = \intval($parent->getAttribute('start')))) { - $number = $start + $element->getSiblingPosition() - 1; + if ($listType === 'ol' && ($parent = $element->getParent()) && \is_numeric($start = $parent->getAttribute('start'))) { + $number = (int) $start + $element->getSiblingPosition() - 1; } else { $number = $element->getSiblingPosition(); } diff --git a/tests/HtmlConverterTest.php b/tests/HtmlConverterTest.php index 4b71832..e62247e 100644 --- a/tests/HtmlConverterTest.php +++ b/tests/HtmlConverterTest.php @@ -192,6 +192,7 @@ class HtmlConverterTest extends TestCase $this->assertHtmlGivesMarkdown('
      1. Item A
      2. Item B
      ', "1. Item A\n2. Item B"); $this->assertHtmlGivesMarkdown('
      1. Item A

        Description

      2. Item B
      ', "1. ### Item A\n \n Description\n2. Item B"); $this->assertHtmlGivesMarkdown('
      1. Item A
      2. Item B
      ', "120. Item A\n121. Item B"); + $this->assertHtmlGivesMarkdown('
      1. Item A
      2. Item B
      ', "0. Item A\n1. Item B"); $this->assertHtmlGivesMarkdown('', "- first item of first list\n- second item of first list\n\n* first item of second list", ['list_item_style_alternate' => '*']); }