fix: read the docs the same on a CRLF checkout - #990
Open
Dusk1e wants to merge 1 commit into
Open
Conversation
npm run test fails on main for anyone whose git checked out CRLF, which is
the default on Windows:
1) public exports
finds the docs it is supposed to read:
named imports of @opensea/seaport-js in README code fences
The fence pattern is anchored on a newline:
/```[a-zA-Z]*\n([\s\S]*?)```/g
so it never matches ```js\r\n, and the README scan comes back with nothing.
CI checks out LF and stays green, which is why this was not visible.
The vacuity guard is what caught it, and it caught what it was written for.
Without it the run would have been green with the two export checks reading
an empty document: both filter a list and compare against [], so an empty
input passes them.
Normalise line endings when the files are read, so every scan sees the same
text whatever the checkout did. importedSymbolsIn also normalises its own
argument, so the parse does not depend on how the caller got the text, and
that is what the new case drives -- the same fence through both line
endings. It is checkout independent, so a CI run on LF catches a regression
here too.
This file contains hidden or 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
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.
Problem
npm run testfails onmainfor anyone whose git checked out CRLF, which is the default on Windows:The fence pattern is anchored on a newline:
So it never matches
```js\r\n, and the README scan comes back empty. Converting README.md to LF and rerunning the same commit turns it green again, which is why CI has not seen this — it checks out LF.The vacuity guard is what caught it, and it caught exactly what it was put there for. Without it the run would have been green while reading nothing:
exports every symbol the README's code examples importandexports every helper a doc comment promises is exposedboth filter a list and compare against[], so an empty input satisfies them. On a CRLF clone those two checks were passing without looking at anything.Fix
Normalise line endings where the files are read, so every scan sees the same text whatever the checkout did.
importedSymbolsInnormalises its own argument too. That is not redundancy for its own sake — it makes the parse independent of how the caller obtained the text, which is what lets it be driven directly from a test.Test
One case, feeding the same fence through both line endings and expecting the same symbol out of each. It does not read from disk, so it holds regardless of what the checkout did, and a CI run on LF catches a regression here as well.
On a CRLF checkout this suite goes from one failure to none; the full run is 244.