add more flexibility to hyphen separated check; explicit em-dash check #185
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WillForan
force-pushed
the
list-item-extra
branch
from
fb036a6
to
72f9229
Compare
WillForan
force-pushed
the
list-item-extra
branch
from
72f9229
to
ae91733
Compare
sindresorhus
requested changes
Jan 1, 2024
rules/list-item.js
Outdated
| continue; | ||
| } | ||
|
|
||
| let [link, ...description] = paragraph.children; | ||
|
|
||
| // Might have children like {image} {text} {link} - {descrition} |
rules/list-item.js
Outdated
| // Might have children like {image} {text} {link} - {descrition} | ||
| // Keep discarding elements until we find a link | ||
| // or linkReference | ||
| if (!/link/.test(link.type)) { |
There was a problem hiding this comment.
Compare explicitly. Doesn't make sense to use regex here.
There was a problem hiding this comment.
besides linkReference and link, I'm not sure if there are other link items in markdown -- figured any type matching "link" should get to pass
link.type !== 'linkReference' && link.type !== 'link' is a mouth full :)
There was a problem hiding this comment.
You could use Array#includes.
besides linkReference and link, I'm not sure if there are other link items in markdown
Look at the docs for the Markdown parser.
rules/list-item.js
Outdated
| // Keep discarding elements until we find a link | ||
| // or linkReference | ||
| if (!/link/.test(link.type)) { | ||
| for (let i = 0; i < description.length - 1; i++) { |
There was a problem hiding this comment.
worse, I don't think I need to test link's type twice. while loop is probably better? (in recent commit)
rules/list-item.js
Outdated
Comment on lines
112
to
113
| // Keep discarding elements until we find a link | ||
| // or linkReference |
There was a problem hiding this comment.
Suggested change
| // Keep discarding elements until we find a link | |
| // or linkReference | |
| // Keep discarding elements until we find a `link` or `linkReference`. |
Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>
Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>
sindresorhus
reviewed
Jan 1, 2024
rules/list-item.js
Outdated
| // NB. We need remark-lint-no-undefined-references separately | ||
| // to catch if this is a valid reference. Here we only care that it exists. | ||
| if (link.type === 'linkReference') { | ||
| // TODO: need to test link children against listItemLinkNodeAllowList? |
WillForan
force-pushed
the
list-item-extra
branch
from
6cb1d02
to
749034a
Compare
WillForan
force-pushed
the
list-item-extra
branch
from
749034a
to
a526a1c
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
test to confirm #146 and #49 are resolved (though I think at least 146 was already making it through without getting flagged)
The big kludge in this pull request is allowing description text matching " - " to pass through instead of getting hit with "link and description must be separated with a dash."
I also un-commented the TODO list-items fixture that had want-to-be valid image-before-link entries.
They had em-dash separators instead of standard hyphens. I updated lint message to complain about that in addition to en-dashes.
Fixes #146
Fixes #49