From 89d0f5b896e4f9d6f0f017aed292abcff961b9ca Mon Sep 17 00:00:00 2001 From: Titus Wormer Date: Mon, 10 Jul 2023 12:11:05 +0200 Subject: [PATCH] Remove `bulletOrderedOther`, always use other bullets Previously, two adjacent lists, which can only be created with markdown by using different bullets, were serialized with the primary bullet, but an empty comment was injected between them. Now, different bullets are always used. Closes GH-56. --- lib/handle/list.js | 21 +---- lib/join.js | 12 --- lib/types.js | 56 +++++------ lib/util/check-bullet-ordered-other.js | 39 -------- readme.md | 49 ++++------ test/index.js | 123 ++++++------------------- 6 files changed, 73 insertions(+), 227 deletions(-) delete mode 100644 lib/util/check-bullet-ordered-other.js diff --git a/lib/handle/list.js b/lib/handle/list.js index 505745e..4bb54cf 100644 --- a/lib/handle/list.js +++ b/lib/handle/list.js @@ -8,7 +8,6 @@ import {checkBullet} from '../util/check-bullet.js' import {checkBulletOther} from '../util/check-bullet-other.js' import {checkBulletOrdered} from '../util/check-bullet-ordered.js' -import {checkBulletOrderedOther} from '../util/check-bullet-ordered-other.js' import {checkRule} from '../util/check-rule.js' /** @@ -25,22 +24,12 @@ export function list(node, parent, state, info) { let bullet = node.ordered ? checkBulletOrdered(state) : checkBullet(state) /** @type {string} */ const bulletOther = node.ordered - ? checkBulletOrderedOther(state) + ? bullet === '.' + ? ')' + : '.' : checkBulletOther(state) - const bulletLastUsed = state.bulletLastUsed - let useDifferentMarker = false - - if ( - parent && - // Explicit `other` set. - (node.ordered - ? state.options.bulletOrderedOther - : state.options.bulletOther) && - bulletLastUsed && - bullet === bulletLastUsed - ) { - useDifferentMarker = true - } + let useDifferentMarker = + parent && state.bulletLastUsed ? bullet === state.bulletLastUsed : false if (!node.ordered) { const firstListItem = node.children ? node.children[0] : undefined diff --git a/lib/join.js b/lib/join.js index a1bbbd3..54e86b6 100644 --- a/lib/join.js +++ b/lib/join.js @@ -20,18 +20,6 @@ function joinDefaults(left, right, parent, state) { return false } - // Two lists with the same marker. - if ( - left.type === 'list' && - left.type === right.type && - Boolean(left.ordered) === Boolean(right.ordered) && - !(left.ordered - ? state.options.bulletOrderedOther - : state.options.bulletOther) - ) { - return false - } - // Join children of a list or an item. // In which case, `parent` has a `spread` field. if ('spread' in parent && typeof parent.spread === 'boolean') { diff --git a/lib/types.js b/lib/types.js index 726285e..6171c51 100644 --- a/lib/types.js +++ b/lib/types.js @@ -299,44 +299,34 @@ * Configuration (optional). * @property {'*' | '+' | '-' | null | undefined} [bullet='*'] * Marker to use for bullets of items in unordered lists (default: `'*'`). - * @property {'*' | '+' | '-' | null | undefined} [bulletOther] - * Marker to use in certain cases where the primary bullet doesn’t work - * (default: depends). * * There are three cases where the primary bullet cannot be used: * - * * When three list items are on their own, the last one is empty, and - * `bullet` is also a valid `rule`: `* - +`. - * This would turn into a thematic break if serialized with three primary - * bullets. - * As this is an edge case unlikely to appear in normal markdown, the - * last list item will be given a different bullet. - * * When a thematic break is the first child of one of the list items, and - * `bullet` is the same character as `rule`: `- ***`. - * This would turn into a single thematic break if serialized with - * primary bullets. - * As this is an edge case unlikely to appear in normal markdown this - * markup is always fixed, even if `bulletOther` is not passed - * * When two unordered lists appear next to each other: `* a\n- b`. - * CommonMark sees different bullets as different lists, but several - * markdown parsers parse it as one list. - * To solve for both, we instead inject an empty comment between the two - * lists: `* a\n\n* b`, but if `bulletOther` is given explicitly, - * it will be used instead + * * when three or more list items are on their own, the last one is empty, + * and `bullet` is also a valid `rule`: `* - +`; this would turn into a + * thematic break if serialized with three primary bullets; `bulletOther` + * is used for the last item + * * when a thematic break is the first child of a list item and `bullet` is + * the same character as `rule`: `- ***`; this would turn into a single + * thematic break if serialized with primary bullets; `bulletOther` is used + * for the item + * * when two unordered lists appear next to each other: `* a\n- b`; + * `bulletOther` is used for such lists + * @property {'*' | '+' | '-' | null | undefined} [bulletOther] + * Marker to use in certain cases where the primary bullet doesn’t work + * (default: `'-'` when `bullet` is `'*'`, `'*'` otherwise). + * + * Cannot be equal to `bullet`. * @property {'.' | ')' | null | undefined} [bulletOrdered='.'] * Marker to use for bullets of items in ordered lists (default: `'.'`). - * @property {'.' | ')' | null | undefined} [bulletOrderedOther] - * Marker to use in certain cases where the primary bullet for ordered items - * doesn’t work (default: none). - * - * There is one case where the primary bullet for ordered items cannot be used: - * - * * When two ordered lists appear next to each other: `1. a\n2) b`. - * CommonMark added support for `)` as a marker, but other markdown - * parsers do not support it. - * To solve for both, we instead inject an empty comment between the two - * lists: `1. a\n\n1. b`, but if `bulletOrderedOther` is given - * explicitly, it will be used instead + * + * There is one case where the primary bullet for ordered items cannot be + * used: + * + * * when two ordered lists appear next to each other: `1. a\n2) b`; to + * solve + * that, `'.'` will be used when `bulletOrdered` is `')'`, and `'.'` + * otherwise * @property {boolean | null | undefined} [closeAtx=false] * Whether to add the same number of number signs (`#`) at the end of an ATX * heading as the opening sequence (default: `false`). diff --git a/lib/util/check-bullet-ordered-other.js b/lib/util/check-bullet-ordered-other.js deleted file mode 100644 index 0e75c4f..0000000 --- a/lib/util/check-bullet-ordered-other.js +++ /dev/null @@ -1,39 +0,0 @@ -/** - * @typedef {import('../types.js').Options} Options - * @typedef {import('../types.js').State} State - */ - -import {checkBulletOrdered} from './check-bullet-ordered.js' - -/** - * @param {State} state - * @returns {Exclude} - */ -export function checkBulletOrderedOther(state) { - const bulletOrdered = checkBulletOrdered(state) - const bulletOrderedOther = state.options.bulletOrderedOther - - if (!bulletOrderedOther) { - return bulletOrdered === '.' ? ')' : '.' - } - - if (bulletOrderedOther !== '.' && bulletOrderedOther !== ')') { - throw new Error( - 'Cannot serialize items with `' + - bulletOrderedOther + - '` for `options.bulletOrderedOther`, expected `*`, `+`, or `-`' - ) - } - - if (bulletOrderedOther === bulletOrdered) { - throw new Error( - 'Expected `bulletOrdered` (`' + - bulletOrdered + - '`) and `bulletOrderedOther` (`' + - bulletOrderedOther + - '`) to be different' - ) - } - - return bulletOrderedOther -} diff --git a/readme.md b/readme.md index eb80f6f..0b1213c 100644 --- a/readme.md +++ b/readme.md @@ -314,50 +314,35 @@ The following fields influence how markdown is serialized. Marker to use for bullets of items in unordered lists (`'*'`, `'+'`, or `'-'`, default: `'*'`). +There are three cases where the primary bullet cannot be used: + +* when three or more list items are on their own, the last one is empty, and + `bullet` is also a valid `rule`: `* - +`; this would turn into a thematic + break if serialized with three primary bullets; `bulletOther` is used for + the last item +* when a thematic break is the first child of a list item and `bullet` is the + same character as `rule`: `- ***`; this would turn into a single thematic + break if serialized with primary bullets; `bulletOther` is used for the + item +* when two unordered lists appear next to each other: `* a\n- b`; + `bulletOther` is used for such lists + ###### `options.bulletOther` Marker to use in certain cases where the primary bullet doesn’t work (`'*'`, -`'+'`, or `'-'`, default: depends). +`'+'`, or `'-'`, default: `'-'` when `bullet` is `'*'`, `'*'` otherwise). -There are three cases where the primary bullet cannot be used: - -* When three list items are on their own, the last one is empty, and `bullet` - is also a valid `rule`: `* - +`. - This would turn into a thematic break if serialized with three primary - bullets. - As this is an edge case unlikely to appear in normal markdown, the last list - item will be given a different bullet. -* When a thematic break is the first child of one of the list items, and - `bullet` is the same character as `rule`: `- ***`. - This would turn into a single thematic break if serialized with primary - bullets. - As this is an edge case unlikely to appear in normal markdown this markup is - always fixed, even if `bulletOther` is not passed -* When two unordered lists appear next to each other: `* a\n- b`. - CommonMark sees different bullets as different lists, but several markdown - parsers parse it as one list. - To solve for both, we instead inject an empty comment between the two lists: - `* a\n\n* b`, but if `bulletOther` is given explicitly, it will be - used instead +Cannot be equal to `bullet`. ###### `options.bulletOrdered` Marker to use for bullets of items in ordered lists (`'.'` or `')'`, default: `'.'`). -###### `options.bulletOrderedOther` - -Marker to use in certain cases where the primary bullet for ordered items -doesn’t work (`'.'` or `')'`, default: none). - There is one case where the primary bullet for ordered items cannot be used: -* When two ordered lists appear next to each other: `1. a\n2) b`. - CommonMark added support for `)` as a marker, but other markdown parsers - do not support it. - To solve for both, we instead inject an empty comment between the two lists: - `1. a\n\n1. b`, but if `bulletOrderedOther` is given explicitly, it - will be used instead +* when two ordered lists appear next to each other: `1. a\n2) b`; to solve + that, `'.'` will be used when `bulletOrdered` is `')'`, and `'.'` otherwise ###### `options.closeAtx` diff --git a/test/index.js b/test/index.js index 11e532d..c9d8ab6 100644 --- a/test/index.js +++ b/test/index.js @@ -86,7 +86,7 @@ test('core', async function (t) { ) await t.test( - 'should inject HTML comments between lists w/ the same marker', + 'should use a different marker for adjacent lists', async function () { assert.equal( to({ @@ -108,7 +108,7 @@ test('core', async function (t) { {type: 'paragraph', children: [{type: 'text', value: 'd'}]} ] }), - 'a\n\n*\n\n\n\n*\n\n1.\n\n\n\n1.\n\nd\n' + 'a\n\n*\n\n-\n\n1.\n\n1)\n\nd\n' ) } ) @@ -3485,7 +3485,7 @@ test('listItem', async function (t) { ]) ) ), - '* * * a\n\n \n\n *\n' + '* * * a\n\n -\n' ) } ) @@ -3523,54 +3523,10 @@ test('listItem', async function (t) { } ) - await t.test('should support `bulletOrderedOther`', async function () { - assert.equal( - to( - { - type: 'root', - children: [ - { - type: 'list', - ordered: true, - children: [{type: 'listItem', children: []}] - }, - { - type: 'list', - ordered: true, - children: [{type: 'listItem', children: []}] - } - ] - }, - {bulletOrdered: '.', bulletOrderedOther: ')'} - ), - '1.\n\n1)\n' - ) - }) - await t.test( - 'should throw on a `bulletOrderedOther` that is invalid', + 'should use a different bullet for adjacent ordered lists', async function () { - assert.throws(function () { - to( - { - type: 'list', - ordered: true, - children: [{type: 'listItem', children: []}] - }, - - { - // @ts-expect-error: check how the runtime handles `bulletOrderedOther`. - bulletOrderedOther: '~' - } - ) - }, /Cannot serialize items with `~` for `options.bulletOrderedOther`/) - } - ) - - await t.test( - 'should throw on a `bulletOrderedOther` that matches `bulletOrdered`', - async function () { - assert.throws(function () { + assert.equal( to( { type: 'root', @@ -3587,9 +3543,10 @@ test('listItem', async function (t) { } ] }, - {bulletOrdered: '.', bulletOrderedOther: '.'} - ) - }, /Expected `bulletOrdered` \(`.`\) and `bulletOrderedOther` \(`.`\) to be different/) + {bulletOrdered: ')'} + ), + '1)\n\n1.\n' + ) } ) }) @@ -4672,107 +4629,83 @@ test('roundtrip', async function (t) { } ) - await t.test( - 'should roundtrip different lists w/ `bulletOrderedOther`', - async function () { - const tree = from('1. a\n1) b') + await t.test('should roundtrip adjacent ordered lists', async function () { + const tree = from('1. a\n1) b') - assert.deepEqual( - removePosition(tree, {force: true}), - removePosition( - from(to(tree, {bulletOrdered: '.', bulletOrderedOther: ')'})), - {force: true} - ) - ) - } - ) + assert.deepEqual( + removePosition(tree, {force: true}), + removePosition(from(to(tree)), {force: true}) + ) + }) await t.test( - 'should roundtrip different lists w/ `bulletOrderedOther` and lists that could turn into thematic breaks (1)', + 'should roundtrip different ordered lists and lists that could turn into thematic breaks (1)', async function () { const tree = from('1. ---\n1) 1. 1)\n1. b') assert.deepEqual( removePosition(tree, {force: true}), - removePosition( - from(to(tree, {bulletOrdered: '.', bulletOrderedOther: ')'})), - {force: true} - ) + removePosition(from(to(tree)), {force: true}) ) } ) await t.test( - 'should roundtrip different lists w/ `bulletOrderedOther` and lists that could turn into thematic breaks (2)', + 'should roundtrip different ordered lists and lists that could turn into thematic breaks (2)', async function () { const tree = from('1. 1. 1)\n1) ---\n1. b') assert.deepEqual( removePosition(tree, {force: true}), - removePosition( - from(to(tree, {bulletOrdered: '.', bulletOrderedOther: ')'})), - {force: true} - ) + removePosition(from(to(tree)), {force: true}) ) } ) await t.test( - 'should roundtrip different lists w/ `bulletOrderedOther` and lists that could turn into thematic breaks (3)', + 'should roundtrip different ordered lists and lists that could turn into thematic breaks (3)', async function () { const tree = from('1. 1. 1)\n1. 1.') assert.deepEqual( removePosition(tree, {force: true}), - removePosition( - from(to(tree, {bulletOrdered: '.', bulletOrderedOther: ')'})), - {force: true} - ) + removePosition(from(to(tree)), {force: true}) ) } ) await t.test( - 'should roundtrip different lists w/ `bulletOrderedOther` and lists that could turn into thematic breaks (4)', + 'should roundtrip different ordered lists and lists that could turn into thematic breaks (4)', async function () { const tree = from('1. 1) 1.\n 1.\n 1)\n 1.') assert.deepEqual( removePosition(tree, {force: true}), - removePosition( - from(to(tree, {bulletOrdered: '.', bulletOrderedOther: ')'})), - {force: true} - ) + removePosition(from(to(tree)), {force: true}) ) } ) await t.test( - 'should roundtrip different lists w/ `bulletOrderedOther` and lists that could turn into thematic breaks (5)', + 'should roundtrip different ordered lists and lists that could turn into thematic breaks (5)', async function () { const tree = from('1. 1) 1.\n 1) 1.\n 1)\n 1.') assert.deepEqual( removePosition(tree, {force: true}), - removePosition( - from(to(tree, {bulletOrdered: '.', bulletOrderedOther: ')'})), - {force: true} - ) + removePosition(from(to(tree)), {force: true}) ) } ) await t.test( - 'should roundtrip different lists w/ `bulletOrderedOther` and lists that could turn into thematic breaks (6)', + 'should roundtrip different ordered lists and lists that could turn into thematic breaks (6)', async function () { const tree = from('1. 1)\n1. 1.\n 1)\n 1.') assert.deepEqual( removePosition(tree, {force: true}), - removePosition( - from(to(tree, {bulletOrdered: '.', bulletOrderedOther: ')'})), - {force: true} - ) + removePosition(from(to(tree)), {force: true}) ) } )