Harmonize list item indent literals

[jablko]

Initial checklist

• I read the support docs
• I read the contributing guide
• I agree to follow the code of conduct
• I searched issues and couldn’t find anything (or linked relevant results below)
• If applicable, I’ve added docs and tests

Description of changes

Whereas remark-stringify and mdast-util-to-markdown accept only 'one', 'tab', or 'mixed', remark-lint-list-item-indent accepts only 'space', 'tab-size', or 'mixed'.

What do you think about allowing them to accept either the current or the preferred literals (whichever those are), and maybe dropping support for all but the preferred literals at some future point?

[wooorm]

This is a breaking change, which I’d rather not do, especially if there’s no good reason for it.

These options are also affected by the proposal (which is a breaking change) of syntax-tree/mdast-util-to-markdown#48, syntax-tree/mdast-util-to-markdown#50, and syntax-tree/mdast-util-to-markdown#51.

[jablko]

@wooorm Is it a breaking change? 'one' are 'tab' are unused values, currently. Tolerating them seems like it might avoid some mistakes, by users configuring both remark-stringify and remark-lint?

[wooorm]

You commit is removing the existing values (tab-size, space), not just adding new values?

[jablko]

No, should be backwards compatible. Maybe support for all but the preferred literals could be dropped at some future point ... Or not.

[wooorm]

Hmm, I guess I missed this line?

Your object is nice but it’s a) a bit hard to read, and b) it’s vulnerable to things like toString or constructor being passed (which here doesn’t seem to result in actual problems, but it’s often good to steer clear of it anyway):
How about:

// To do in major: remove legacy fallbacks.
if (option === 'tab-size') {
  option = 'tab'
} else if (option === 'space') {
  option === 'one'
}

[jablko]

• I didn't think of that (other properties of "empty" objects), thanks! I've replaced it with:

    option =
      option === 'tab-size' ? 'tab'
        : option === 'space' ? 'one'
        : option

[wooorm]

For some reason one comment in the review didn’t come through (I think?):

I think I prefer the existing (this rule’s) values instead of the remark-stringify ones. They’re more about that this rule checks for a size, instead of an actual tab character or so. What do you think?

[jablko]

I see your point about tab-size vs. tab. I was motivated by the inconsistency, not the actual values --- this PR doesn't make sense if the values aren't preferred, especially in view of syntax-tree/mdast-util-to-markdown#51.

I'll close this and subscribe to syntax-tree/mdast-util-to-markdown#51 instead. Thanks!

[github-actions]

Hi! This was closed. Team: If this was merged, please describe when this is likely to be released. Otherwise, please add one of the no/* labels.