ordered-list-marker-value: fix to support 0 as start

[alexandrtovmach]

Here's small patch that allow to use 0 as start value in ordered list for https://github.com/remarkjs/remark-lint/tree/master/packages/remark-lint-ordered-list-marker-value rule.

Why is that need? Because lists like that should be valid:

0. zero
1. one
2. two
3. three

but it's not with current implementation:

Marker should be 2, was 1 ordered-list-marker-value remark-lint

Example

I'm working now on remark lint preset for Gatsby docs and faced with this issue here:

That's not a BREAKING CHANGE and tests seems okay

[wooorm]

I think this would mostly work! I would feel a bit more comfortable that this isn’t a breaking change with something that checks if node.start is null/undefined, something like this (if allowed by xo/prettier though):

    var shouldBe = pref === 'one' ? 1 : node.start == null ? 1 : node.start

[alexandrtovmach]

@wooorm Not sure that I understand your point =) I just tried to avoid some critical changes, and simply replaced start value. Do you want me to refactor this like you said?

[wooorm]

Yes, that’s what I meant!

With your proposal, if node.start is undefined or null, it should be 0. With my suggestion, that case should be 1 instead. But both allow node.start to be 0 (which you are solving)!

[alexandrtovmach]

@wooorm Updated with strict equality instead of node.start == null. But... I still not sure about that, because new logic is can't handle all cases like NaN and empty string:

var values = [undefined, null, 0, 1, NaN, ''];
var pref = 'one';

function test1(valuesArr) {
	return valuesArr.map(el => (pref === 'one' ? 1 : el) || 0)
};


function test2(valuesArr) {
	function getStartValue(value) {
        if (typeof value !== "undefined" && value !== null) {
            return value
        } else {
            return 0;
        }
    }
    return valuesArr.map(el => pref === 'one' ? 1 : getStartValue(el))
};

console.log(test1(values)); // [1, 1, 1, 1, 1, 1]
console.log(test2(values)); // [1, 1, 1, 1, 1, 1]
var pref = 'ordered';
console.log(test1(values)); // [0, 0, 0, 1, 0, 0]
console.log(test2(values)); // [0, 0, 0, 1, NaN, ""]

[alexandrtovmach]

@wooorm Could you take a look?

[alexandrtovmach]

@wooorm Friendly ping

[wooorm]

Thanks for the pings and sorry for not getting back to you sooner!

[wooorm]

We don’t have to care about weird values, because node.start should be a number, but it is optional! So null and undefined are both allowed, according to mdast.

Here is a script I wrote to compare the three algorithms. Algorithm A is the one I proposed, B is your earlier proposal, and C your latest proposal

var values = [null, undefined, 0, 1] // Values that `node.start` could have
var preferences = ['single', 'ordered', 'one'] // Options for the rule

preferences.forEach(p => {
  console.log('\npreference: %s', p)

  values.forEach(function(d) {
    var a = p === 'one' ? 1 : d == null ? 1 : d
    var b = (p === 'one' ? 1 : d) || 0
    var c = p === 'one' ? 1 : typeof d !== 'undefined' && d !== null ? d : 0

    console.log('%s: %s | %s | %s', String(d).padStart(10), a, b, c)
  })
})

This gives the result:

preference: single
      null: 1 | 0 | 0
 undefined: 1 | 0 | 0
         0: 0 | 0 | 0
         1: 1 | 1 | 1

preference: ordered
      null: 1 | 0 | 0
 undefined: 1 | 0 | 0
         0: 0 | 0 | 0
         1: 1 | 1 | 1

preference: one
      null: 1 | 1 | 1
 undefined: 1 | 1 | 1
         0: 1 | 1 | 1
         1: 1 | 1 | 1

In these results, we can ignore the one preferences. Those are all the same.
We can also ignore single OR ordered, because they have the same results. So now we just care about, let’s say, single.

Your algorithms now all prefer to start with 0. This may be a breaking change because sometimes nodes don’t have a start, and used to result in a 1.
With my proposed algorithm A, null and undefined are treated as before (1), whereas only an explicit 0 results in a 0!

[alexandrtovmach]

@wooorm Thank you for detailed explanation. Updated code with your algorithm

[wooorm]

Sweet, this is great!

One more nit 😬 Could you add a test? Basically these and these lines, above or below those, but (starting) with 0s? Thanks!

[wooorm]

Perfect! 😊

[alexandrtovmach]

@wooorm I can't find this changes on NPM. Does it released?

[wooorm]

Released! https://github.com/remarkjs/remark-lint/releases/tag/ordered-list-marker-value%401.0.4